Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tool to mark an API version as unserved #229

Merged
merged 3 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions tools/crd-bumper/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
Expand Down
6 changes: 5 additions & 1 deletion tools/crd-bumper/pkg/conversion_gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand All @@ -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?
Expand Down
130 changes: 130 additions & 0 deletions tools/crd-bumper/pkg/unserve.py
Original file line number Diff line number Diff line change
@@ -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)
141 changes: 141 additions & 0 deletions tools/crd-bumper/unserve.py
Original file line number Diff line number Diff line change
@@ -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())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to wrap this in a try/except block since you raise some exceptions below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I have a need to pretty-up the error, I'm going to leave it.



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)
Loading