Upgrade Kanister CRDs to v1 from v1beta1 #927
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
viveksinghggits
changed the title
We tried to use kubebuilder to generate the CRDs for
go types but that is also not able to process some of
the types, for example `BlueprintPhase.Args`, `Phase.Output`
becauaes of `interface{}` and `Blueprint.Actions` because of
pointer type value.
- Remove required fields from CRDs - Move CR resource names to consts package to resolve import cycle issue - Remove un-nec logging
Even though e2e tests were running on my local cluster tests were failing on the kanister CI, and the reason was, when we read blueprint to create blueprint type if a field is not specified in the blueprint, we initialized the type with null for that field and in that case CRD validation failed. This commit tries to fix that by settign `omitemtpy` for those fields. I am thinking if we should add `omitempty` for every field in types.go
In the test `ResourceSuite.TestBlueprintClient` we tried to create blueprint with empty actions struct and then tried to make sure that we are able to update the blueprint. And checked actions after update that its not nil. Since we are omitting empty field we are not going to get actions if we pass empty actions, that why I have changed the action to have some values.
viveksinghggits
commented
Jun 3, 2021
Comment on lines
-182
to
-186
| const ( | ||
| BlueprintResourceName = "blueprint" | ||
| BlueprintResourceNamePlural = "blueprints" | ||
| ) | ||
|
|
There was a problem hiding this comment.
I have moved these to consts package to resolve import cycle issue that we faced when we tried to use them from customresource package.
|
Per current implementation if CRDs are already created in the cluster nothing would happen. But in our case we would like to re-create them with I am continuing looking into this. |
Per current implementation we were not silently ignoring the error if the CRDs are already present in the cluster. Since we are significantly changing the CRD that wouldn't work for, so this PR updates the CRDs if they are already avaialable.
pavannd1
reviewed
Jun 24, 2021
pavannd1
reviewed
Jun 24, 2021
Hi @pavannd1 |
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.
Change Overview
This PR tries to update the version of CRDs that we create to
v1fromv1beta1. Some of the issue that I facedrequiredbut we don't necessarily provide all the fields while creating the CRs. So we had to remove thoserequiredfields from CRDs.After above changes, things were working on cluster version 1.20, but in kanister CI we run kubernetes version 1.18, and there things were failing because when we read the blueprint from
examples/dir to actually marshal it to types json, the values that we didnt specify in the manifest were being set asnull. To resolve that we had to addomitemptyin the types.go for some fields.Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan
Change
integration_test.shto run the test forWith the Kubernetes version that runs in CI
logs can be found here.
With kubernetes version 1.20
and the logs can be found here.