From 275206d9bf6b44dd768b2f125ba0106df06a2d1f Mon Sep 17 00:00:00 2001 From: Dean Roehrich Date: Thu, 14 Nov 2024 14:18:08 -0600 Subject: [PATCH 1/2] A tool to mark an API version as unserved. Signed-off-by: Dean Roehrich --- tools/crd-bumper/README.md | 19 +++- tools/crd-bumper/pkg/conversion_gen.py | 6 +- tools/crd-bumper/pkg/unserve.py | 130 +++++++++++++++++++++++ tools/crd-bumper/unserve.py | 141 +++++++++++++++++++++++++ 4 files changed, 294 insertions(+), 2 deletions(-) create mode 100644 tools/crd-bumper/pkg/unserve.py create mode 100755 tools/crd-bumper/unserve.py diff --git a/tools/crd-bumper/README.md b/tools/crd-bumper/README.md index db7d86f..382aef3 100644 --- a/tools/crd-bumper/README.md +++ b/tools/crd-bumper/README.md @@ -67,13 +67,30 @@ The following example will vendor the new `v1beta2` API we created above for lus ```console DEST_REPO=git@github.com:NearNodeFlash/nnf-sos.git -vendor-new-api.py -r --hub-ver v1alpha3 $DEST_REPO --vendor-hub-ver v1beta2 --module github.com/NearNodeFlash/lustre-fs-operator --version master +vendor-new-api.py -r $DEST_REPO --hub-ver v1alpha3 --vendor-hub-ver v1beta2 --module github.com/NearNodeFlash/lustre-fs-operator --version master ``` The repository with its new API will be found under a directory named `workingspace/nnf-sos`. The new `api-lustre-fs-operator-v1beta2` branch will have a commit containing the newly-vendored API and adjusted code. This commit message will have **ACTION** comments describing something that must be manually verified, and possibly adjusted, before the tests will succeed. +## Removing an Old API Version + +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. + +### Unserve the API + +The following example will mark the old `v1alpha1` API in lustre-fs-operator as no longer being served. This will place a `+kubebuilder:unservedversion` in each CRD of that version, which `controller-gen` will translate 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 will do all of its work. + +```console +REPO=git@github.com:NearNodeFlash/lustre-fs-operator.git +unserve.py -r $REPO --spoke-ver v1alpha1 +``` + +The repository with its new API will be found under a directory named `workingspace/lustre-fs-operator`. + +The new `api-v1alpha1-unserve` branch will have a commit containing the adjusted API and adjusted code. This commit message will have **ACTION** comments describing something that must be manually verified, and possibly adjusted, before the tests will succeed. + ## Library and Tool Support 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.) diff --git a/tools/crd-bumper/pkg/conversion_gen.py b/tools/crd-bumper/pkg/conversion_gen.py index 0153263..877fbc1 100644 --- a/tools/crd-bumper/pkg/conversion_gen.py +++ b/tools/crd-bumper/pkg/conversion_gen.py @@ -36,7 +36,7 @@ def __init__(self, dryrun, project, prev_ver, new_ver, most_recent_spoke): self._most_recent_spoke = most_recent_spoke self._module = None self.module() - self._preferred_alias = self.preferred_api_alias() + self._preferred_alias = self.set_preferred_api_alias() def is_spoke(self, ver): """ @@ -52,6 +52,10 @@ def is_spoke(self, ver): return True def preferred_api_alias(self): + """Return the preferred alias.""" + return self._preferred_alias + + def set_preferred_api_alias(self): """ Is this repo using the API "group" as the alias or is it using something else? diff --git a/tools/crd-bumper/pkg/unserve.py b/tools/crd-bumper/pkg/unserve.py new file mode 100644 index 0000000..298c4a3 --- /dev/null +++ b/tools/crd-bumper/pkg/unserve.py @@ -0,0 +1,130 @@ +# Copyright 2024 Hewlett Packard Enterprise Development LP +# Other additional copyright holders may be indicated within. +# +# The entirety of this work is licensed under the Apache License, +# Version 2.0 (the "License"); you may not use this file except +# in compliance with the License. +# +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os +import re + +from .fileutil import FileUtil + + +class Unserve: + """Tools to mark an API as unserved.""" + + def __init__(self, dryrun, project, spoke_ver, preferred_alias): + self._dryrun = dryrun + self._project = project + self._spoke_ver = spoke_ver + self._preferred_alias = self.set_preferred_alias(preferred_alias) + + def set_preferred_alias(self, preferred_alias): + """ + Take the preferred alias if it's provided, else grab it from the + first Kind of this API. + """ + if preferred_alias is None: + # Pick the first kind, use its group. + kinds = self._project.kinds(self._spoke_ver) + group = self._project.group(kinds[0], self._spoke_ver) + else: + group = preferred_alias + return group + + def set_unserved(self): + """ + Set the kubebuilder:unservedversion marker in the specified spoke API, + for any Kind that does not yet have it. + """ + + kinds = self._project.kinds(self._spoke_ver) + for kind in kinds: + fname = f"api/{self._spoke_ver}/{kind.lower()}_types.go" + if os.path.isfile(fname): + fu = FileUtil(self._dryrun, fname) + found = fu.find_in_file("+kubebuilder:unservedversion") + if found: + continue + # Prefer to pair with kubebuilder:subresource:status, but fall back + # to kubebuilder:object:root=true if status cannot be found. + line = fu.find_in_file("+kubebuilder:subresource:status") + if line is None: + line = fu.find_in_file("+kubebuilder:object:root=true") + if line is None: + raise LookupError( + f"Unable to place kubebuilder:unservedversion in {fname}" + ) + fu.replace_in_file(line, f"""{line}\n// +kubebuilder:unservedversion""") + fu.store() + + def modify_conversion_webhook_suite_test(self): + """ + Modify the suite test that exercises the conversion webhook. + + Update the tests for the specified spoke API. + + Recall that this verifies that the conversion routines are accessed via + the conversion webhook, and that is not intended to be an exhaustive + conversion test. + + WARNING WARNING: This contains a series of multi-line Python f-strings + which contain large chunks of Go code. So it's confusing. Stay sharp. + """ + + conv_go = "internal/controller/conversion_test.go" + if not os.path.isfile(conv_go): + print(f"NOTE: Unable to find {conv_go}!") + return + + fu = FileUtil(self._dryrun, conv_go) + + # An ACTION note to be added to each test that we think should be removed. + action_note = f"// ACTION: {self._spoke_ver} is no longer served, and this test can be removed." + # Pattern to find the "It()" method so we can change it to "PIt()". + pat = r"^(\s+)It(\(.*)" + kinds = self._project.kinds(self._spoke_ver) + for kind in kinds: + spec = fu.find_in_file( + f"""It("reads {kind} resource via hub and via spoke {self._spoke_ver}", func()""" + ) + if spec is not None: + newspec = spec + m = re.search(pat, spec) + if m is not None: + newspec = f"{m.group(1)}PIt{m.group(2)}" + + # Wake up! Multi-line f-string: + template = f""" + It("is unable to read {kind} resource via spoke {self._spoke_ver}", func() {{ + resSpoke := &{self._preferred_alias}{self._spoke_ver}.{kind}{{}} + Expect(k8sClient.Get(context.TODO(), client.ObjectKeyFromObject(resHub), resSpoke)).ToNot(Succeed()) + }}) +""" # END multi-line f-string. + + fu.replace_in_file(spec, f"{template}\n{newspec}\n{action_note}\n") + fu.store() + + def commit(self, git, stage): + """Create a commit message.""" + + msg = f"""Mark the {self._spoke_ver} API as unserved. + +ACTION: Address the ACTION comments in internal/controller/conversion_test.go. + +ACTION: Begin by running "make vet". Repair any issues that it finds. + Then run "make test" and continue repairing issues until the tests + pass. +""" + git.commit_stage(stage, msg) diff --git a/tools/crd-bumper/unserve.py b/tools/crd-bumper/unserve.py new file mode 100755 index 0000000..1d321f2 --- /dev/null +++ b/tools/crd-bumper/unserve.py @@ -0,0 +1,141 @@ +#!/usr/bin/env python3 + +# Copyright 2024 Hewlett Packard Enterprise Development LP +# Other additional copyright holders may be indicated within. +# +# The entirety of this work is licensed under the Apache License, +# Version 2.0 (the "License"); you may not use this file except +# in compliance with the License. +# +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Mark the specified CRD version as unserved.""" + +import argparse +import sys + +from pkg.conversion_gen import ConversionGen +from pkg.git_cli import GitCLI +from pkg.make_cmd import MakeCmd +from pkg.project import Project +from pkg.unserve import Unserve + +WORKING_DIR = "workingspace" +BRANCH_SUFFIX = "unserve" + +PARSER = argparse.ArgumentParser() +PARSER.add_argument( + "--spoke-ver", + type=str, + required=True, + help="Spoke API version to mark as unserved.", +) +PARSER.add_argument( + "--repo", + "-r", + type=str, + required=True, + help="Git repository URL which has the Go code that provides the APIs.", +) +PARSER.add_argument( + "--branch", + "-b", + type=str, + required=False, + help=f"Branch name to create. Default is 'api--{BRANCH_SUFFIX}'", +) +PARSER.add_argument( + "--this-branch", + action="store_true", + dest="this_branch", + help="Continue working in the current branch.", +) +PARSER.add_argument( + "--dry-run", + "-n", + action="store_true", + dest="dryrun", + help="Dry run. Implies only one step.", +) +PARSER.add_argument( + "--no-commit", + "-C", + action="store_true", + dest="nocommit", + help="Skip git-commit. Implies only one step.", +) +PARSER.add_argument( + "--workdir", + type=str, + required=False, + default=WORKING_DIR, + help=f"Name for working directory. All repos will be cloned below this directory. Default: {WORKING_DIR}.", +) + + +def main(): + """main""" + + args = PARSER.parse_args() + + gitcli = GitCLI(args.dryrun, args.nocommit) + gitcli.clone_and_cd(args.repo, args.workdir) + + project = Project(args.dryrun) + + cgen = ConversionGen(args.dryrun, project, args.spoke_ver, None, None) + makecmd = MakeCmd(args.dryrun, None, None, None) + + if args.branch is None: + args.branch = f"api-{args.spoke_ver}-{BRANCH_SUFFIX}" + if args.this_branch: + print("Continuing work in current branch") + else: + print(f"Creating branch {args.branch}") + try: + gitcli.checkout_branch(args.branch) + except RuntimeError as ex: + print(str(ex)) + print( + "If you are continuing in an existing branch, then specify `--this-branch`." + ) + sys.exit(1) + + if not cgen.is_spoke(args.spoke_ver): + print(f"API --spoke-ver {args.spoke_ver} is not a spoke.") + sys.exit(1) + + unserve_api(args, project, makecmd, gitcli, cgen.preferred_api_alias()) + + +def unserve_api(args, project, makecmd, git, preferred_api_alias): + """Mark the specified API version as unserved.""" + + unserve = Unserve(args.dryrun, project, args.spoke_ver, preferred_api_alias) + + print(f"Updating files to mark API {args.spoke_ver} as unserved.") + + unserve.set_unserved() + unserve.modify_conversion_webhook_suite_test() + + makecmd.manifests() + makecmd.generate() + makecmd.generate_go_conversions() + makecmd.fmt() + makecmd.clean_bin() + + unserve.commit(git, "unserve-api") + + +if __name__ == "__main__": + main() + +sys.exit(0) From 1b516ba6a5ba042e9f53f64b4799ad876735d60d Mon Sep 17 00:00:00 2001 From: Dean Roehrich Date: Thu, 14 Nov 2024 14:20:20 -0600 Subject: [PATCH 2/2] ug Signed-off-by: Dean Roehrich --- tools/crd-bumper/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/crd-bumper/README.md b/tools/crd-bumper/README.md index 382aef3..0f84238 100644 --- a/tools/crd-bumper/README.md +++ b/tools/crd-bumper/README.md @@ -87,7 +87,7 @@ REPO=git@github.com:NearNodeFlash/lustre-fs-operator.git unserve.py -r $REPO --spoke-ver v1alpha1 ``` -The repository with its new API will be found under a directory named `workingspace/lustre-fs-operator`. +The repository with its adjusted API will be found under a directory named `workingspace/lustre-fs-operator`. The new `api-v1alpha1-unserve` branch will have a commit containing the adjusted API and adjusted code. This commit message will have **ACTION** comments describing something that must be manually verified, and possibly adjusted, before the tests will succeed.