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

doc: Add hint that the userID must not include the client. prefix #5072

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

Mtze
Copy link
Contributor

@Mtze Mtze commented Jan 10, 2025

Describe what this PR does

This PR only adds a small hint in the rbd helm values chart. It took me quite some time to find my config error and this hint hopefully safes someone some debug time :)

Problem was that i included the client. prefix in the userID secret.

Related issues

Mention any github issues relevant to this PR. Adding below line
will help to auto close the issue once the PR is merged.

Problem mentioned in: #4304

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

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 bug Something isn't working ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures component/docs Issues and PRs related to documentation labels Jan 10, 2025
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.

Can you please make similar comment in https://github.com/ceph/ceph-csi/blob/devel/examples/cephfs/secret.yaml and https://github.com/ceph/ceph-csi/blob/devel/examples/rbd/secret.yaml and drop other commit related to controller-runtime update

go.mod Outdated
@@ -42,7 +42,7 @@ require (
k8s.io/mount-utils v0.31.3
k8s.io/pod-security-admission v0.31.3
k8s.io/utils v0.0.0-20240711033017-18e509b52bc8
sigs.k8s.io/controller-runtime v0.19.3
sigs.k8s.io/controller-runtime v0.19.4
Copy link
Member

Choose a reason for hiding this comment

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

not sure how this got included, please keep the change to the userKey only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fork was out of sync :)

@Mtze Mtze force-pushed the devel branch 3 times, most recently from c4ad526 to 4e70d98 Compare January 14, 2025 15:04
@Mtze
Copy link
Contributor Author

Mtze commented Jan 14, 2025

done :)

@Madhu-1 Madhu-1 requested a review from nixpanic January 15, 2025 08:05
@nixpanic
Copy link
Member

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Jan 15, 2025

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

@Mergifyio queue

Copy link
Contributor

mergify bot commented Jan 15, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at bdd7c5a

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jan 15, 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/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

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

@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.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/mini-e2e/k8s-1.31

@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-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jan 15, 2025
@mergify mergify bot merged commit bdd7c5a into ceph:devel Jan 15, 2025
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ci/skip/e2e skip running e2e CI jobs ci/skip/multi-arch-build skip building on multiple architectures component/docs Issues and PRs related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants