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.

Sign up for GitHub

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

Jump to bottom

helm: Optionally set userID and userKey in cephfs chart. #4801

Merged
merged 1 commit into from
Aug 28, 2024
Merged

helm: Optionally set userID and userKey in cephfs chart. #4801

merged 1 commit into from
Aug 28, 2024

Conversation

james-choncholas
Copy link
Contributor

The cephfs static provisioner expect userID and userKey in the credential secret. This PR adds these values to the helm chart so that they are only included in the templated yaml if the values are non-empty.

Describe what this PR does

This PR adds userID and userKey to the cephfs provisioner helm chart. These credentials are required for static provisioning. The userID and userKey are optional values in values.yaml. If they are not provided, they are excluded from the secret.

Is the change backward compatible? Yes

Are there concerns around backward compatibility? No

Related issues

#4467

Future concerns

None

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes
    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.
Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@mergify mergify bot added the component/deployment Helm chart, kubernetes templates and configuration Issues/PRs label Aug 25, 2024
@iPraveenParihar iPraveenParihar mentioned this pull request Aug 26, 2024
Closed
Madhu-1
Madhu-1 reviewed Aug 26, 2024
View reviewed changes
Copy link
Collaborator

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

small nit

charts/ceph-csi-cephfs/values.yaml Outdated
@@ -331,6 +331,9 @@ secret:
# specified in the storage class
adminID: <plaintext ID>
adminKey: <Ceph auth key corresponding to ID above>
# User credentials are required for the static provisioner.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# User credentials are required for the static provisioner.
# User credentials are required for the static provisioned PVC.

can you please update the same above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I fixed this occurrence and the two others in the chart README.

@Rakshith-R
Copy link
Contributor

Please check commitlint failure as well.
https://github.com/ceph/ceph-csi/actions/runs/10550648436/job/29243552546?pr=4801

@nixpanic nixpanic added ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures and removed ci/skip/e2e skip running e2e CI jobs labels Aug 27, 2024
@nixpanic nixpanic requested a review from Madhu-1 August 27, 2024 12:15
@Madhu-1 Madhu-1 requested a review from a team August 28, 2024 09:44
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Aug 28, 2024

@Mergifyio queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 28, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The pull request cannot be checked because of an incompatibility with branch protections.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Aug 28, 2024

@Mergifyio rebase

@james-choncholas @Madhu-1
helm: optionally set userID and userKey in cephfs chart
61d5bbf 
According to ceph#4467 the cephfs
static provisioner expect userID and userKey in the credential secret.
Add these values to the helm chart so that they are only included in the
templated yaml if the values are non-empty.

Signed-off-by: james-choncholas <jim@choncholas.com>
@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 28, 2024

rebase

✅ Branch has been successfully rebased

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Aug 28, 2024

@Mergifyio queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 28, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 3fbe7a8

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Aug 28, 2024
@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/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.31

@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.31

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Aug 28, 2024
@mergify mergify bot merged commit 3fbe7a8 into ceph:devel Aug 28, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yati1998 yati1998 yati1998 approved these changes

@Madhu-1 Madhu-1 Madhu-1 approved these changes

Assignees
No one assigned
Labels
ci/skip/multi-arch-build skip building on multiple architectures component/deployment Helm chart, kubernetes templates and configuration Issues/PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@james-choncholas @Rakshith-R @Madhu-1 @ceph-csi-bot @yati1998 @nixpanic