-
Notifications
You must be signed in to change notification settings - Fork 25
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
Update K8s library versions to v0.20.4 #37
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These libs are being updated so new functionality which relies upon the K8s API can be added to cluster-controller. This commit does not re-generate the turndown CRDs, I wanted to separate the commits for some clarity. The build fails on this commit.
Without this rewrite, the old script failed with: Generating deepcopy funcs F0211 10:52:30.681397 71752 deepcopy.go:885] Hit an unsupported type invalid type for invalid type, from github.com/kubecost/cluster-turndown/pkg/apis/turndownschedule/v1alpha1.TurndownSchedule Initially I found this issue: operator-framework/operator-sdk#1854 and tried to use the GOROOT fix, but it didn't work. I ended up getting (with set -x): $ GOROOT=/usr/bin bash generate.sh ... + bash ./vendor/k8s.io/code-generator/generate-groups.sh all github.com/kubecost/cluster-turndown/pkg/generated github.com/kubecost/cluster-turndown/pkg/apis turndownschedule:v1alpha1 --output-base . --go-header-file hack/custom-boilerplate.go.txt /home/delta/go/pkg/mod/golang.org/x/tools@v0.0.0-20210106214847-113979e3529a/internal/imports/imports.go:12:2: package bufio is not in GOROOT (/usr/bin/src/bufio) /home/delta/go/pkg/mod/github.com/spf13/pflag@v1.0.5/flag.go:102:2: package bytes is not in GOROOT (/usr/bin/src/bytes) ... etc. I opted to rewrite the generate.sh script in the image of the official K8s sample-controller, with some additions based on the previous generate.sh described in comments. I'm not sure if the go mod vendor trick in the old generate.sh, which purportedly got around some trouble caused when the code wasn't yet generated, will be necessary to resurrect in the future if we update the API version. I'll document this in the PR as well, so hopefully someone can find this with some spelunking if the problem arises again. After the rewrite, generation with "bash generate.sh" succeeds. Go build is now failing because we need to add contexts to our K8s API calls, which is expected after the library version update. Generate code with correct imports
This is necessary because the update of the K8s libs means calls require contexts and, in some cases, a new options struct. After this commit, "go build cmd/turndown/main.go" succeeds.
…e other build directive for future proofing.
mbolt35
approved these changes
Feb 16, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks great to me. I'm ok with the generate.sh
changes given the absurd complexities of kubernetes resource generation.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm working on adding some functionality to cluster-controller and I want to have newer versions of the K8s libs, which means turndown also needs to be updated with the new versions.
Updating the K8s libraries necessitated regenerating the CRD code, which is what most of the effort in this PR is dedicated to. Also, calls to the K8s API had to be updated with contexts and occasionally new options structs. I used
context.TODO()
for all calls that now require contexts.There's quite a bit of extra detail in commit messages and comments.
@mbolt35 I rewrote generate.sh while I was debugging, and it ended up really similar to what you already had. I'm able to avoid doing the vendor voodoo in the old generate.sh script because the code has already been generated once AND because I don't delete
./pkg/generated
before runninggo mod vendor
like the old script does. If you think I should port my findings back to the old generate script, I'm fine with doing that. Let me know and let's chat to make sure we're on the same page.TODO: PR to follow this one updating the version of the turndown images to v1.3.0.
Testing
Got a fun, unrelated issue out of testing this: #38
Once I removed the PDB in my cluster, testing went smoothly. Test cluster is on GKE and has a single, non-autoscaling node pool.
Pre-turndown:
Post-turndown
Post-turnup