Skip to content

Commit ffefafd

Browse files
roehrich-hpeajfloeder
roehrich-hpe
and
ajfloeder
authored
Add crd-bumper docs for bumping CRD versions (#246)
Signed-off-by: Dean Roehrich <dean.roehrich@hpe.com> Co-authored-by: Tony Floeder <anthony.floeder@hpe.com>
1 parent 3066755 commit ffefafd

5 files changed

Lines changed: 333 additions & 11 deletions

File tree

‎docs/repo-guides/crd-bumper/editing-apis.md‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# Editing APIs
2+
3+
Tips, tutorials, examples, best practices.
4+
5+
## Tips
6+
7+
Some helpful tips to make life easier while writing conversion routines.
8+
9+
* Do not delete fields. It's easier to write loss-less conversion routines if the field is renamed to something that indicates it's no longer used. This becomes more important in the spoke-hub-spoke scenario, because the hub will not have an annotation preserving the original spoke's values.
10+
11+
## Example 1: Add a field to a resource
12+
13+
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`.
14+
15+
### Step 1: Add a new ResourceError field
16+
17+
```go
18+
diff --git a/api/v1alpha2/systemconfiguration_types.go b/api/v1alpha2/systemconfiguration_types.go
19+
index 3e4d29fb..2c65e3a0 100644
20+
--- a/api/v1alpha2/systemconfiguration_types.go
21+
+++ b/api/v1alpha2/systemconfiguration_types.go
22+
@@ -82,6 +82,9 @@ type SystemConfigurationSpec struct {
23+
type SystemConfigurationStatus struct {
24+
// Ready indicates when the SystemConfiguration has been reconciled
25+
Ready bool `json:"ready"`
26+
+
27+
+ // Error information
28+
+ ResourceError `json:",inline"`
29+
}
30+
31+
//+kubebuilder:object:root=true
32+
```
33+
34+
(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.)
35+
36+
### Step 2: Run the conversion code generator
37+
38+
Run the generator to update the generated conversion routines for this type.
39+
40+
```console
41+
make generate-go-conversions
42+
```
43+
44+
This will emit some scary-looking output:
45+
46+
```bash
47+
/Users/droehrich/work/rabsw/dws-7.git/bin/conversion-gen \
48+
--output-file=zz_generated.conversion.go \
49+
--go-header-file=./hack/boilerplate.go.txt \
50+
./api/v1alpha1
51+
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
52+
E0913 11:13:26.993609 23295 conversion.go:742] the following fields need manual conversion:
53+
E0913 11:13:26.993613 23295 conversion.go:744] - ResourceError
54+
```
55+
56+
Let's look around for a missing routine:
57+
58+
```console
59+
$ git status
60+
On branch api-change-demo
61+
Changes not staged for commit:
62+
(use "git add <file>..." to update what will be committed)
63+
(use "git restore <file>..." to discard changes in working directory)
64+
modified: api/v1alpha1/zz_generated.conversion.go
65+
modified: api/v1alpha2/systemconfiguration_types.go
66+
```
67+
68+
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:
69+
70+
```console
71+
$ git diff -U1 api/v1alpha1/zz_generated.conversion.go
72+
diff --git a/api/v1alpha1/zz_generated.conversion.go b/api/v1alpha1/zz_generated.conversion.go
73+
index 1f1b3cb6..d7c915aa 100644
74+
--- a/api/v1alpha1/zz_generated.conversion.go
75+
+++ b/api/v1alpha1/zz_generated.conversion.go
76+
@@ -2082,2 +2082,3 @@ func autoConvert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurat
77+
out.Ready = in.Ready
78+
+ // WARNING: in.ResourceError requires manual conversion: does not exist in peer-type
79+
return nil
80+
@@ -2085,7 +2086,2 @@ func autoConvert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurat
81+
82+
-// Convert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus is an autogenerated conversion function.
83+
-func Convert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus(in *v1alpha2.SystemConfigurationStatus, out *SystemConfigurationStatus, s conversion.Scope) error {
84+
- return autoConvert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus(in, out, s)
85+
-}
86+
-
87+
func autoConvert_v1alpha1_SystemConfigurationStorageNode_To_v1alpha2_SystemConfigurationStorageNode(in *SystemConfigurationStorageNode, out *v1alpha2.SystemConfigurationStorageNode, s conversion.Scope) error {
88+
```
89+
90+
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.
91+
92+
### Step 3: Restore the generated conversion entrypoint
93+
94+
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` succeeds.
95+
96+
```console
97+
$ git diff -U1 api/v1alpha1/conversion.go
98+
diff --git a/api/v1alpha1/conversion.go b/api/v1alpha1/conversion.go
99+
index f409f9b2..e11a41eb 100644
100+
--- a/api/v1alpha1/conversion.go
101+
+++ b/api/v1alpha1/conversion.go
102+
@@ -581,2 +581,6 @@ func Convert_v1alpha2_SystemConfigurationSpec_To_v1alpha1_SystemConfigurationSpe
103+
104+
+func Convert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus(in *dwsv1alpha2.SystemConfigurationStatus, out *SystemConfigurationStatus, s apiconversion.Scope) error {
105+
+ return autoConvert_v1alpha2_SystemConfigurationStatus_To_v1alpha1_SystemConfigurationStatus(in, out, s)
106+
+}
107+
+
108+
func Convert_v1alpha2_StorageSpec_To_v1alpha1_StorageSpec(in *dwsv1alpha2.StorageSpec, out *StorageSpec, s apiconversion.Scope) error {
109+
```
110+
111+
Confirm that it compiles:
112+
113+
```console
114+
make vet
115+
```
116+
117+
### Step 4: Add the new field to the conversion routine
118+
119+
Add this field to the `ConvertTo()` (convert from spoke to hub) conversion routine for the `SystemConfiguration` type.
120+
121+
```console
122+
$ git diff -U1 api/v1alpha1/conversion.go
123+
diff --git a/api/v1alpha1/conversion.go b/api/v1alpha1/conversion.go
124+
index f409f9b2..706f16bb 100644
125+
--- a/api/v1alpha1/conversion.go
126+
+++ b/api/v1alpha1/conversion.go
127+
@@ -369,2 +369,3 @@ func (src *SystemConfiguration) ConvertTo(dstRaw conversion.Hub) error {
128+
dst.Spec.PortsCooldownInSeconds = restored.Spec.PortsCooldownInSeconds
129+
+ dst.Status.Error = restored.Status.Error
130+
131+
```
132+
133+
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.
134+
135+
Try the test suite, but make it easy on yourself and try `make vet` first:
136+
137+
```console
138+
make vet
139+
make test
140+
```

‎docs/repo-guides/crd-bumper/readme.md‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
# CRD Version Bumper
2+
3+
Bump the CRD version, adding conversion webhooks and tests.
4+
5+
See Kubebuilder's [Tutorial: Multi-Version API](https://book.kubebuilder.io/multiversion-tutorial/tutorial) for a description of the mechanism. For more detail read the Kubernetes document [Versions in CustomResourceDefinitions](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/).
6+
7+
## Hub and Spokes
8+
9+
This tool implements the hub-and-spoke model of CRD versioning. See Kubebuilder's [Hubs, spokes, and other wheel metaphors](https://book.kubebuilder.io/multiversion-tutorial/conversion-concepts) for a description of the model.
10+
11+
The new CRD version will be the new hub, and the previous hub will become a new spoke. (Note: in the Kubebuilder book example, the old version is the hub and never changes, and all new versions are spokes.)
12+
13+
## Using CRD Bumper
14+
15+
Create the environment prior to running the tool:
16+
17+
```console
18+
$ python3 -m venv venv
19+
$ . venv/bin/activate
20+
(venv) $ pip install -r requirements.txt
21+
```
22+
23+
### Prior to Creating the new API
24+
25+
For the easiest vendoring experience, the downstream repos should be up to date with this repo prior to creating the new API, and this new API should be vendored into the downstream repos before any further changes go into this repo or any of the downstream repos.
26+
27+
### Run the Tool
28+
29+
Clone a fresh copy of the repository that contains the CRDs and controllers, checking out to the default branch (master or main). The tool expects a repository that is compatible with kubebuilder and will use the `./PROJECT` file that is maintained by kubebuilder.
30+
31+
The following example creates a new API version `v1beta2` for the lustre-fs-operator repository, where `v1beta1` is the existing hub and `v1alpha1` is the most recent existing spoke. It begins by creating a new branch off "master" named `api-v1beta2`, where it does all of its work.
32+
33+
```console
34+
REPO=git@github.com:NearNodeFlash/lustre-fs-operator.git
35+
crd-bumper.py --repo $REPO --most-recent-spoke v1alpha1 --prev-ver v1beta1 --new-ver v1beta2 all
36+
```
37+
38+
The repository with its new API is found under a directory named `workingspace/lustre-fs-operator`.
39+
40+
The new `api-v1beta2` branch has a series of commits showing a progression of steps. Some of these commit messages have an **ACTION** comment describing something that must be manually verified, and possibly adjusted, before the tests succeed.
41+
42+
### Verification
43+
44+
After the entire progression of steps has completed, verify the results by running `make vet`, and paying attention to any of the **ACTION** comments described above. Once `make vet` is clean, move to the next debug step by running `make test`.
45+
46+
Do not run `make vet` or `make test` before the entire progression of steps has completed. **The individual commits do not build--the whole set of commits is required.**
47+
48+
### Stepping
49+
50+
Sometimes it can be helpful to do the steps one at a time. If the first step has not yet been done, then begin by using the `step` command in place of the `all` command. It begins, as with the `all` command, by creating a new branch off `master` named `api-v1beta2`, where it will do all of its work.
51+
52+
```console
53+
crd-bumper.py --repo $REPO --most-recent-spoke v1alpha1 --prev-ver v1beta1 --new-ver v1beta2 step
54+
```
55+
56+
Follow that with more steps, telling the tool to continue in the current branch that was created during the first step by adding `--this-branch`. The other args **must** remain the same as they were on the first command. The tool knows when there are no more steps to be done.
57+
58+
```console
59+
crd-bumper.py --repo $REPO --most-recent-spoke v1alpha1 --prev-ver v1beta1 --new-ver v1beta2 --this-branch step
60+
```
61+
62+
Do not attempt to run `make vet` or `make test` between steps. The individual commits do not build.
63+
64+
## Vendor the New API
65+
66+
Vendor this new API into another repository using the `vendor-new-api` tool. This tool updates that repo to change its Go code references and its Kustomize config references to point at the new API.
67+
68+
### Executing the Tool
69+
70+
The following example vendors the new `v1beta2` API we created above for lustre-fs-operator into the nnf-sos repository. The current hub version for nnf-sos is `v1alpha3`, and is specified with the `--hub-ver` option. The module representing lustre-fs-operator is specified in the same form that it would appear in the `go.mod` file in nnf-sos. The `--version` option can be used to specify a version if necessary. It begins by creating a new branch in nnf-sos off "master" named `api-lustre-fs-operator-v1beta2`, where it does all of its work.
71+
72+
```console
73+
DEST_REPO=git@github.com:NearNodeFlash/nnf-sos.git
74+
vendor-new-api.py -r $DEST_REPO --hub-ver v1alpha3 --vendor-hub-ver v1beta2 --module github.com/NearNodeFlash/lustre-fs-operator --version master
75+
```
76+
77+
The repository with its new API is found under a directory named `workingspace/nnf-sos`.
78+
79+
The new `api-lustre-fs-operator-v1beta2` branch has a commit containing the newly-vendored API and adjusted code. This commit message has **ACTION** comments describing something that must be manually verified, and possibly adjusted, before the tests succeed.
80+
81+
## Removing an Old API Version
82+
83+
An old API version should first be shipped in a deprecated state. Use the `unserve` tool to mark that API version as no longer being served by the API server. After that has shipped, that version of the API can be removed in a later release.
84+
85+
### Unserve the API
86+
87+
The following example marks the old `v1alpha1` API in lustre-fs-operator as no longer being served. This places a `+kubebuilder:unservedversion` in each CRD of that version, which `controller-gen` translates into `served: false` for that version when it regenerates the CRD manifest. It begins by creating a new branch in lustre-fs-operator off "master" named `api-v1alpha1-unserve`, where it does all of its work.
88+
89+
```console
90+
REPO=git@github.com:NearNodeFlash/lustre-fs-operator.git
91+
unserve.py -r $REPO --spoke-ver v1alpha1
92+
```
93+
94+
The repository with its adjusted API is found under a directory named `workingspace/lustre-fs-operator`.
95+
96+
The new `api-v1alpha1-unserve` branch has a commit containing the adjusted API and adjusted code. This commit message has **ACTION** comments describing something that must be manually verified, and possibly adjusted, before the tests succeed.
97+
98+
## Library and Tool Support
99+
100+
The library and tool support is taken from the [Cluster API](https://github.com/kubernetes-sigs/cluster-api) project. See [release v1.6.6](https://github.com/kubernetes-sigs/cluster-api/tree/release-1.6) for a version that contains multi-version support for CRDs where they have a hub with one spoke. (Note: In v1.7.0 they removed the old API--the old spoke--and their repo contains only one version, the hub.)
101+
102+
In the "References" section you'll find a link to a video from the Cluster API team where they go through an example of changing an API and providing conversion routines and tests for the change.
103+
104+
### Lossless Conversions
105+
106+
The libraries from the Cluster API project use an annotation on the spoke version of a resource to contain a serialized copy of the hub version of that resource. The libraries and tests use this to avoid data loss during `hub->spoke->hub` conversions.
107+
108+
## Writing Conversion Routines
109+
110+
### Conversion Library Lifecycle in Spoke APIs
111+
112+
After an API version has been bumped, the spoke API should be frozen. However, the spoke API conversion library in `api/<spoke-ver>/conversion.go` continues to be updated, following the progression and development of the hub.
113+
114+
### Conversion Must Not Fail
115+
116+
When writing conversion routines be aware that the conversion routine must not report a failure:
117+
118+
"Failing conversion can disrupt read and write access to the custom resources, including the ability to update or delete the resources. Conversion failures should be avoided whenever possible, and should not be used to enforce validation constraints (use validation schemas or webhook admission instead)." Kubernetes "Versions in CustomResourceDefinitions" [Response](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#response).
119+
120+
## Code Markers
121+
122+
This tool uses "marker comments" in the Go code to indicate where generated code should be placed.
123+
These markers are used the same way `controller-gen` uses code markers, as documented in Kubebuilder's [Markers for Config/Code Generation](https://book.kubebuilder.io/reference/markers).
124+
125+
The following markers must be present in the code prior to using this tool:
126+
127+
**+crdbumper:scaffold:builder**
128+
129+
Used in `internal/controller/suite_test.go` to indicate where additional calls to `SetupWebhookWithManager` should be placed. This should be in the `BeforeSuite()` function before the first reconciler is set up.
130+
131+
**+crdbumper:scaffold:gvk**
132+
133+
Used in `github/cluster-api/util/conversion/conversion_test.go` to indicate where additional `schema.GroupVersionKind{}` types should be placed.
134+
135+
**+crdbumper:scaffold:marshaldata**
136+
137+
Used in `github/cluster-api/util/conversion/conversion_test.go` to indicate where additional marshalling tests should be placed. This should be the last statement in the `TestMarshalData()` function.
138+
139+
**+crdbumper:scaffold:unmarshaldata**
140+
141+
Used in `github/cluster-api/util/conversion/conversion_test.go` to indicate where additional unmarshalling tests should be placed. This should be the last statement in the `TestUnmarshalData()` function.
142+
143+
**+crdbumper:scaffold:webhooksuitetest**
144+
145+
Used in `internal/controller/conversion_test.go` to indicate where additional tests for a new Kind may be placed. This should be the last statement within the main `Describe()` block.
146+
147+
**+crdbumper:scaffold:spoketest="GROUP.KIND"**
148+
149+
Used in `internal/controller/conversion_test.go` to indicate where additional spoke conversion tests should be placed. Each kind will have its own `Context()` block within the main `Describe()`, and this should be the last statement within each of those Context blocks. Replace "GROUP.KIND" with the API's group and kind, using the same spelling and use of lower/upper case letters as found in the `./PROJECT` file.
150+
151+
**+crdbumper:carryforward:begin="KIND.DIRECTION"**
152+
153+
**+crdbumper:carryforward:begin="Epilog"**
154+
155+
Used in `api/$SPOKE/conversion.go` to indicate that a segment of code should be carried forward as new spokes are created or as old spokes are removed. Use of this marker should be uncommon; it's almost always wrong to carry-forward code to a new spoke. Replace "KIND" with the API's kind, using the same spelling and use of lower/upper case letters as found in the `./PROJECT` file. Replace "DIRECTION" with "ConvertFrom" or "ConvertTo" to mark the section of code as being in a `ConvertFrom()` or `ConvertTo()` function. If the "Epilog" variation is used then this marks one or more `Convert_$API1_$KIND_To_$API2_$KIND()` conversion functions, or similar chunks of code, that are typically collected at the end of `conversion.go`.
156+
157+
**+crdbumper:carryforward:end**
158+
159+
This is used in `api/$SPOKE/conversion.go` to close the segment of code that was marked with `+crdbumper:carryforward:begin`.
160+
161+
## References
162+
163+
### Kubernetes
164+
165+
[Versions in CustomResourceDefinitions](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/)
166+
167+
### Kubebuilder
168+
169+
[Tutorial: Multi-Version API](https://book.kubebuilder.io/multiversion-tutorial/tutorial)
170+
171+
[Hubs, spokes, and other wheel metaphors](https://book.kubebuilder.io/multiversion-tutorial/conversion-concepts)
172+
173+
### Cluster API
174+
175+
[Cluster API release 1.6 branch](https://github.com/kubernetes-sigs/cluster-api/tree/release-1.6)
176+
177+
Video: [SIG Cluster Lifecycle - ClusterAPI - API conversion code walkthrough](https://www.youtube.com/watch?v=Mk14N4SelNk)

‎docs/repo-guides/readme.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,8 @@
33
## Management
44

55
* [Releasing NNF Software](release-nnf-sw/readme.md)
6+
7+
## Developer
8+
9+
* [CRD Version Bumper](crd-bumper/readme.md)
10+
* [Editing APIs](crd-bumper/editing-apis.md)

0 commit comments

Comments
 (0)