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

helm: drop snapshot.storage.k8s.io/v1beta1 #12051

Merged
merged 1 commit into from
Apr 10, 2023

Conversation

sathieu
Copy link
Contributor

@sathieu sathieu commented Apr 8, 2023

Description of your changes:

drop snapshot.storage.k8s.io/v1beta1, v1 exists since k8s v1.20.

Which issue is resolved by this Pull Request:
Resolves #

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide).
  • Skip Tests for Docs: If this is only a documentation change, add the label skip-ci on the PR.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next minor release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@sathieu
Copy link
Contributor Author

sathieu commented Apr 8, 2023

I had a problem with ArgoCD, where rook-ceph was installed (or at leased evaluated) before csi-snapshotter CRDs were installed.

Message is: no matches for kind "VolumeSnapshotClass" in version "snapshot.storage.k8s.io/v1beta1".

(From: https://gitlab.com/kubitus-project/kubitus-installer/-/jobs/4082126134/artifacts/external_file/kubectl_describe_applications.log
https://gitlab.com/kubitus-project/kubitus-installer/-/jobs/4082126134, from https://gitlab.com/kubitus-project/kubitus-installer/-/merge_requests/1214)

@sathieu sathieu changed the title ceph: drop snapshot.storage.k8s.io/v1beta1 helm: drop snapshot.storage.k8s.io/v1beta1 Apr 8, 2023
v1 exists since k8s v1.20

Signed-off-by: Mathieu Parent <mathieu.parent@insee.fr>
@travisn
Copy link
Member

travisn commented Apr 10, 2023

The change looks good to me since K8s 1.21+ is required for Rook v1.11, just one question before merging...

I had a problem with ArgoCD, where rook-ceph was installed (or at leased evaluated) before csi-snapshotter CRDs were installed.

@sathieu Are you saying this was a different problem, or because of these v1beta1 obsolete CRDs?

@sathieu
Copy link
Contributor Author

sathieu commented Apr 10, 2023

@travisn

@sathieu Are you saying this was a different problem, or because of these v1beta1 obsolete CRDs?

This is the problem which leads me to create this PR.

ArgoCD uses helm template and passes available APIs using the --api-versions flag. This helm template command is only run once for a given Git commit (unless "force-refresh" is done). If the snapshot CRDs are not available when helm template is run, the chart will default to CRDs "beta1". ArgoCD will not regenerate the resources, and those will fail to apply, even if snapshot CRDs are available later.

My PR simply removes snapshot v1beta1 CRDs, because they are very old (1.17 <= K8s <= v1.19).

@travisn travisn merged commit 85d7920 into rook:master Apr 10, 2023
47 of 50 checks passed
travisn added a commit that referenced this pull request Apr 11, 2023
helm: drop snapshot.storage.k8s.io/v1beta1 (backport #12051)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants