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

feat(labels): add part-of selector label #211

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

andrewazores
Copy link
Member

  • feat(labels): add part-of selector label
  • chore(service): rename ports to remove 'cryostat-' prefix

Related to cryostatio/cryostat-openshift-console-plugin#2

@andrewazores
Copy link
Member Author

The ct.bash --upgrade tests fail with:

>>> helm upgrade cryostat-8sjfi4ytca charts/cryostat --namespace helm-test --reuse-values --wait --timeout=600s
Error: UPGRADE FAILED: cannot patch "cryostat-8sjfi4ytca-v4" with kind Deployment: Deployment.apps "cryostat-8sjfi4ytca-v4" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/component":"cryostat", "app.kubernetes.io/instance":"cryostat-8sjfi4ytca", "app.kubernetes.io/name":"cryostat", "app.kubernetes.io/part-of":"cryostat"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && cannot patch "cryostat-8sjfi4ytca-v4-db" with kind Deployment: Deployment.apps "cryostat-8sjfi4ytca-v4-db" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/component":"db", "app.kubernetes.io/instance":"cryostat-8sjfi4ytca", "app.kubernetes.io/name":"cryostat", "app.kubernetes.io/part-of":"cryostat"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable && cannot patch "cryostat-8sjfi4ytca-v4-storage" with kind Deployment: Deployment.apps "cryostat-8sjfi4ytca-v4-storage" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/component":"storage", "app.kubernetes.io/instance":"cryostat-8sjfi4ytca", "app.kubernetes.io/name":"cryostat", "app.kubernetes.io/part-of":"cryostat"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable

tl;dr the label selectors are immutable, so upgrading the Deployment in-place can't happen. I think this is kind of failure is OK so long as it only happens on semver major versions when breakages are expected (see #202).

$ bash ct.bash 2>&1 | tee ct.log
$ bash ct.bash --upgrade 2>&1 | tee ct-upgrade.log
$ diff ct.log ct-upgrade.log

@andrewazores andrewazores marked this pull request as ready for review November 20, 2024 19:14
@andrewazores andrewazores requested a review from a team November 20, 2024 19:14
Copy link
Member

@ebaron ebaron left a comment

Choose a reason for hiding this comment

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

We already have some unreleased label selector changes right? So I think the upgrade should still work after we renamed the deployment here like we did in the operator?

@andrewazores
Copy link
Member Author

^ #197 and #202

I think a 3.0 -> 4.0 upgrade will work because of this renaming, but if I understand how the upgrade test works correctly, it's trying to upgrade from main to this PR - so essentially 4.0 to 4.0, and there is no renaming happening between versions, hence the apparent immutability conflict.

@ebaron
Copy link
Member

ebaron commented Nov 21, 2024

^ #197 and #202

I think a 3.0 -> 4.0 upgrade will work because of this renaming, but if I understand how the upgrade test works correctly, it's trying to upgrade from main to this PR - so essentially 4.0 to 4.0, and there is no renaming happening between versions, hence the apparent immutability conflict.

Yup that sounds totally fine to me.

@andrewazores andrewazores merged commit f6127ef into cryostatio:main Nov 21, 2024
5 of 7 checks passed
@andrewazores andrewazores deleted the service-name-labels branch November 21, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants