Tool to mark an API version as unserved (#229)

Signed-off-by: Dean Roehrich <[email protected]>

tools/crd-bumper/README.md

@@ -74,6 +74,23 @@ The repository with its new API will be found under a directory named `workingsp
 
 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
+[email protected]:NearNodeFlash/lustre-fs-operator.git
+unserve.py -r $REPO --spoke-ver v1alpha1
+```
+
+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.
+
 ## 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.)

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?

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)

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-<spoke_ver>-{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)