Add conversion v1 v2 #423
Closed
Add conversion v1 v2 #423
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
dvaldivia
requested review from
harshavardhana,
Alevsk,
nitisht and
cesnietor
dvaldivia
force-pushed
the
add-conversion-v1-v2
branch
2 times, most recently
from
ba415d4
to
d019b25
Compare
harshavardhana
requested changes
Jan 19, 2021
harshavardhana
requested changes
Jan 20, 2021
dvaldivia
force-pushed
the
add-conversion-v1-v2
branch
2 times, most recently
from
364c0eb
to
8119bc8
Compare
|
@harshavardhana agree, testing is needed, I've been doing it myself, and the one that is the most important is testing that a user of |
nitisht
suggested changes
Jan 21, 2021
nitisht
suggested changes
Jan 21, 2021
There was a problem hiding this comment.
I followed these steps:
- Deploy v3.0.29 Operator
- Create a tenant
- Patch the operator pod with new image based on this PR with this command
kubectl patch pod minio-operator-6f5b8cdcff-vkjj7 --type='json' -p='[{"op": "replace", "path": "/spec/containers/0/image", "value":"nitishtiwari/operator:v1v2"}]'
- Create a fresh tenant --> Fails
- Upgrade the existing tenant to a new image --> Fails
|
@nitisht I just fixed it in my latest commit, can you try your steps again? |
|
To test: 1.- Install an older Operator Now wait for the tenant to be ready 3.- Create a tenant that uses zones with apiVersion The previous tenant and the new tenants, all should work cc @harshavardhana @nitisht @Alevsk |
I am getting this error for apiVersion |
|
With |
Tested, worked for me using latest code in thus branch |
Alevsk
suggested changes
Jan 23, 2021
Yes, working for me as well. |
nitisht
previously approved these changes
Jan 25, 2021
|
What is the process (if any) for updating a tenant on the v1 API to v2? |
|
@ravindk89 the idea is that it will magically work, if you have the tenant v1 from |
|
@dvaldivia good to know. Is this something the user needs to check the status on? I'd like to provide simple commands for checking the API status of a tenant after upgrading the operator. |
|
This is hitting us really hard kubernetes-sigs/controller-tools#448 |
Co-authored-by: Lenin Alevski <alevsk.8772@gmail.com>
Co-authored-by: Lenin Alevski <alevsk.8772@gmail.com>
Co-authored-by: Lenin Alevski <alevsk.8772@gmail.com>
Co-authored-by: Lenin Alevski <alevsk.8772@gmail.com>
Co-authored-by: Lenin Alevski <alevsk.8772@gmail.com>
Alevsk
previously approved these changes
Jan 27, 2021
harshavardhana
previously approved these changes
Jan 28, 2021
|
I created a tenant using operator v3.0.28: |
|
creating a second tenant with latest console with deps fixed causes: Seems crd is still specting zones. That is a response from kubernetes. @dvaldivia ^ |
harshavardhana
force-pushed
the
add-conversion-v1-v2
branch
from
78aeebc
to
0ce833e
Compare
nitisht
reviewed
Jan 28, 2021
| required: | ||
| - spec | ||
| type: object | ||
| served: true |
There was a problem hiding this comment.
We should also add the v1 deprecation warning like this https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#version-deprecation
# This indicates the v1alpha1 version of the custom resource is deprecated.
# API requests to this version receive a warning header in the server response.
deprecated: true
# This overrides the default warning returned to API clients making v1alpha1 API requests.
deprecationWarning: "example.com/v1alpha1 CronTab is deprecated; see http://example.com/v1alpha1-v1 for instructions to migrate to example.com/v1 CronTab"
There was a problem hiding this comment.
good point, I'll look into adding the deprecated fields, I can see it's not supported by controller-gen so I'll need to patch it
There was a problem hiding this comment.
done, also sent a PR to controller-gen kubernetes-sigs/controller-tools#541
|
|
||
| // Console Related Constants | ||
|
|
||
| // DefaultConsoleImage specifies the latest Console Docker hub image | ||
| const DefaultConsoleImage = "minio/console:v0.4.6" | ||
| const DefaultConsoleImage = "minio/console:v0.3.14" |
Old tenant (v1) that gets migrated to v2 never gets to running state - since the operator is expecting a new statefulset name but the statefulset name in the tenant remains the same. Pods no longer have resolvable names. |
|
@nitisht just fixed the issue you ran into |
|
error with tenant created using console yaml: Also during this it killed one pod and started failing searching for a secret eventhough tls is disabled for that tenant. |
why are we still using console v0.4.3 ? @cesnietor |
|
Squashed and merged to master 7e532e6 |
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.
Bumps the recent changes to Tenant to
v2api version, and introduces a conversion webhook so that the k8s API can convert between the versions and keep compatibility.Updates all code to work with v2 by default.