From fb4c7b7f36d57d6b4e6c1a6401c49afa980477ce Mon Sep 17 00:00:00 2001 From: Dean Roehrich Date: Fri, 13 Sep 2024 13:50:05 -0500 Subject: [PATCH] Begin doc for examples of modifying APIs (#213) Signed-off-by: Dean Roehrich --- tools/crd-bumper/Editing_APIs.md | 135 +++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 tools/crd-bumper/Editing_APIs.md diff --git a/tools/crd-bumper/Editing_APIs.md b/tools/crd-bumper/Editing_APIs.md new file mode 100644 index 0000000..477b86e --- /dev/null +++ b/tools/crd-bumper/Editing_APIs.md @@ -0,0 +1,135 @@ +# Editing APIs + +Tips, tutorials, examples, best practices. + +## Example 1: Add a field to a resource + +In the DWS repository, add a new field to the `Status` section of the `SystemConfiguration` **hub** resource. In this case, the hub version is `v1alpha2`. + +### Step 1: Add a new ResourceError field + + +```go +diff --git a/api/v1alpha2/systemconfiguration_types.go b/api/v1alpha2/systemconfiguration_types.go +index 3e4d29fb..2c65e3a0 100644 +--- a/api/v1alpha2/systemconfiguration_types.go ++++ b/api/v1alpha2/systemconfiguration_types.go +@@ -82,6 +82,9 @@ type SystemConfigurationSpec struct { + type SystemConfigurationStatus struct { + // Ready indicates when the SystemConfiguration has been reconciled + Ready bool `json:"ready"` ++ ++ // Error information ++ ResourceError `json:",inline"` + } + + //+kubebuilder:object:root=true + ``` + +(An aside: That `ResourceError` type used for this new field is inlined, bringing a field named `Error`, so that's what we'll see next in the conversion routines.) + +### Step 2: Run the conversion code generator + +Run the generator to update the generated conversion routines for this type. + +```console +make generate-go-conversions +``` + +This will emit some scary-looking output: + +```bash +/Users/droehrich/work/rabsw/dws-7.git/bin/conversion-gen \ + --output-file=zz_generated.conversion.go \ + --go-header-file=./hack/boilerplate.go.txt \ + ./api/v1alpha1 +E0913 11:13:26.993464 23295 conversion.go:741] Warning: could not find nor generate a final Conversion function for github.com/DataWorkflowServices/dws/api/v1alpha2.SystemConfigurationStatus -> github.com/DataWorkflowServices/dws/api/v1alpha1.SystemConfigurationStatus +E0913 11:13:26.993609 23295 conversion.go:742] the following fields need manual conversion: +E0913 11:13:26.993613 23295 conversion.go:744] - ResourceError +``` + +Let's look around for a missing routine: + +```console +$ git status +On branch api-change-demo +Changes not staged for commit: + (use "git add ..." to update what will be committed) + (use "git restore ..." to discard changes in working directory) + modified: api/v1alpha1/zz_generated.conversion.go + modified: api/v1alpha2/systemconfiguration_types.go +``` + +Look at the generated code for a clue. There are two here. First, a comment about our new field requiring manual conversion. Second, the generated entrypoint for the `SystemConfigurationStatus` conversion routine has been taken away: + +```console +$ git diff -U1 api/v1alpha1/zz_generated.conversion.go +diff --git a/api/v1alpha1/zz_generated.conversion.go b/api/v1alpha1/zz_generated.conversion.go +index 1f1b3cb6..d7c915aa 100644 +--- a/api/v1alpha1/zz_generated.conversion.go ++++ b/api/v1alpha1/zz_generated.conversion.go +@@ -2082,2 +2082,3 @@ func autoConvert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurat + out.Ready = in.Ready ++ // WARNING: in.ResourceError requires manual conversion: does not exist in peer-type + return nil +@@ -2085,7 +2086,2 @@ func autoConvert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurat + +-// Convert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus is an autogenerated conversion function. +-func Convert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus(in *v1alpha2.SystemConfigurationStatus, out *SystemConfigurationStatus, s conversion.Scope) error { +- return autoConvert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus(in, out, s) +-} +- + func autoConvert_v1alpha1_SystemConfigurationStorageNode_To_v1alpha2_SystemConfigurationStorageNode(in *SystemConfigurationStorageNode, out *v1alpha2.SystemConfigurationStorageNode, s conversion.Scope) error { +``` + +The generator is forcing us to acknowledge that we have conversion covered for that new field. We have to manually replace that entrypoint to hook up the conversion again. + +### Step 3: Restore the generated conversion entrypoint + +At the end of `api/v1alpha1/conversion.go` we will find other entrypoint routines like this one, for earlier changes to the `SystemConfiguration` type. Cut-n-paste that entrypoint routine from the diff above into an appropriate place near the end of `conversion.go`, adjusting it just enough so `make vet` will succeed. + +```console +$ git diff -U1 api/v1alpha1/conversion.go +diff --git a/api/v1alpha1/conversion.go b/api/v1alpha1/conversion.go +index f409f9b2..e11a41eb 100644 +--- a/api/v1alpha1/conversion.go ++++ b/api/v1alpha1/conversion.go +@@ -581,2 +581,6 @@ func Convert_v1alpha2_SystemConfigurationSpec_To_v1alpha1_SystemConfigurationSpe + ++func Convert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus(in *dwsv1alpha2.SystemConfigurationStatus, out *SystemConfigurationStatus, s apiconversion.Scope) error { ++ return autoConvert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus(in, out, s) ++} ++ + func Convert_v1alpha2_StorageSpec_To_v1alpha1_StorageSpec(in *dwsv1alpha2.StorageSpec, out *StorageSpec, s apiconversion.Scope) error { +``` + +Confirm that it will compile: + +```console +make vet +``` + +### Step 4: Add the new field to the conversion routine + +Add this field to the `ConvertTo()` (convert from spoke to hub) conversion routine for the `SystemConfiguration` type. + +```console +$ git diff -U1 api/v1alpha1/conversion.go +diff --git a/api/v1alpha1/conversion.go b/api/v1alpha1/conversion.go +index f409f9b2..706f16bb 100644 +--- a/api/v1alpha1/conversion.go ++++ b/api/v1alpha1/conversion.go +@@ -369,2 +369,3 @@ func (src *SystemConfiguration) ConvertTo(dstRaw conversion.Hub) error { + dst.Spec.PortsCooldownInSeconds = restored.Spec.PortsCooldownInSeconds ++ dst.Status.Error = restored.Status.Error + +``` + +In this case, we won't add this field to the `ConvertFrom()` (convert from hub to spoke) routine because we don't need to represent it in any way in the spoke. This will not always be the case, so do what makes sense for your particular change. + +Try the test suite, but make it easy on yourself and try `make vet` first: + +```console +make vet +make test +```