Skip to content

Conversation

@black-dragon74
Copy link
Member

Describe what this PR does

This patch marks userID and userKey required in helm values.

A release note has been added for the same as well.

Ref: #5282

@mergify mergify bot added the component/deployment Helm chart, kubernetes templates and configuration Issues/PRs label Apr 22, 2025
@black-dragon74 black-dragon74 force-pushed the helm-compat-user-creds branch from cf3c7ec to 0da5296 Compare April 22, 2025 08:09

## Breaking Changes

- Support for `secret.adminID` and `secret.adminKey` has been deprecated and removed.
Copy link
Member

Choose a reason for hiding this comment

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

I think it is only removed from the Helm charts? It is not a breaking change if users did not use Helm to deploy? That could be explained a little better.

@black-dragon74 black-dragon74 force-pushed the helm-compat-user-creds branch from 0da5296 to c992645 Compare April 22, 2025 11:39
@black-dragon74 black-dragon74 requested a review from nixpanic April 22, 2025 11:39
Comment on lines +17 to +18
userID: {{ required "A valid userID must be specified in secret" .Values.secret.userID }}
userKey: {{ required "A valid userKey must be specified in secret" .Values.secret.userKey }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we add adminID and adminKey back in this release and deprecate it and Remove it in next major release?

thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would make sense actually. This got overlooked in the previous release. Breaking changes should not be a part of minor/patch versions.

@black-dragon74 black-dragon74 requested a review from Madhu-1 April 22, 2025 14:30
@nixpanic
Copy link
Member

Does this report the new required message, even if adminID is set in the values.yaml of the current (before attempting to update) deployment?

@black-dragon74
Copy link
Member Author

Does this report the new required message, even if adminID is set in the values.yaml of the current (before attempting to update) deployment?

Yes.

@nixpanic
Copy link
Member

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Apr 22, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 20b9ecc

This patch marks `userID` and `userKey` required in
helm values.

A release note has been added for the same as well.

Signed-off-by: Niraj Yadav <[email protected]>
@mergify mergify bot force-pushed the helm-compat-user-creds branch from c992645 to 209c624 Compare April 23, 2025 07:01
@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Apr 23, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Apr 23, 2025
@mergify mergify bot merged commit 20b9ecc into ceph:devel Apr 23, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/deployment Helm chart, kubernetes templates and configuration Issues/PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants