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: Always add nodes read permissions to provisioner ClusterRole #4798

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

dragoangel
Copy link
Contributor

Describe what this PR does

--immediate-topology=false not stopping registry.k8s.io/sig-storage/csi-provisioner from attemps list nodes, like --feature-gates=Topology=false do, because of errors in csi-provisioner container will paper and it will not work. More details are described here: #4790 (comment)

This PR adds permissions to allow csi-provisioner list nodes and csinodes even when .Values.topology.domainLabels is [] which fixes mentioned above issue.

Is there anything that requires special attention

Do you have any questions?
No

Is the change backward compatible?
Yes

Are there concerns around backward compatibility?
No

Provide any external context for the change, if any.
N/A

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.

properly fixes: #4777

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 the component/deployment Helm chart, kubernetes templates and configuration Issues/PRs label Aug 22, 2024
@dragoangel
Copy link
Contributor Author

@Madhu-1 @iPraveenParihar can you please review?

Copy link
Contributor

@iPraveenParihar iPraveenParihar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@Madhu-1 Madhu-1 added the backport-to-release-v3.12 Label to backport from devel to release-v3.12 branch label Aug 26, 2024
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Aug 26, 2024

@Mergifyio queue

Copy link
Contributor

mergify bot commented Aug 26, 2024

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

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.

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Aug 26, 2024
@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-helm/k8s-1.29

@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/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

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

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

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Aug 26, 2024
@Rakshith-R Rakshith-R added the ci/retry/e2e Label to retry e2e retesting on approved PR's label Aug 26, 2024
@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

@dragoangel "ci/centos/mini-e2e-helm/k8s-1.31" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

@dragoangel "ci/centos/mini-e2e/k8s-1.31" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

@dragoangel "ci/centos/upgrade-tests-cephfs" test failed. Logs are available at location for debugging

Copy link
Contributor

mergify bot commented Aug 27, 2024

dequeue

✅ The pull request has been removed from the queue default

@nixpanic
Copy link
Member

@Mergifyio queue

Copy link
Contributor

mergify bot commented Aug 27, 2024

queue

🛑 The pull request has been removed from the queue default

The pull request can't be updated.

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.

@nixpanic
Copy link
Member

@Mergifyio rebase

Signed-off-by: Dmytro Alieksieiev <1865999+dragoangel@users.noreply.github.com>
Copy link
Contributor

mergify bot commented Aug 27, 2024

rebase

✅ Branch has been successfully rebased

@nixpanic nixpanic force-pushed the fix-rbac-ceph-csi-rbd-provisioner branch from eff81d5 to 7229c3a Compare August 27, 2024 07:30
@nixpanic
Copy link
Member

@Mergifyio queue

Copy link
Contributor

mergify bot commented Aug 27, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 601c40b

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Aug 27, 2024
@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-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

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

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

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Aug 27, 2024
@mergify mergify bot merged commit 601c40b into ceph:devel Aug 27, 2024
36 of 37 checks passed
@dragoangel dragoangel deleted the fix-rbac-ceph-csi-rbd-provisioner branch August 31, 2024 18:50
@HaveFun83
Copy link

Hi any estimate when there will be a new chartrelease which include this fix?

@Bengrunt
Copy link
Contributor

Bengrunt commented Sep 6, 2024

Hi any estimate when there will be a new chartrelease which include this fix?

Yes please, it's breaking our deployments :/

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Sep 6, 2024

Will do a release on next Monday

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Sep 6, 2024

cephcsi 3.12.2 release is out https://github.com/ceph/ceph-csi/releases/tag/v3.12.2 with this change

@HaveFun83
Copy link

@Madhu-1 thanks a lot 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-release-v3.12 Label to backport from devel to release-v3.12 branch ci/retry/e2e Label to retry e2e retesting on approved PR's component/deployment Helm chart, kubernetes templates and configuration Issues/PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ceph-csi 3.12.0 not able to provision RBD PVC.
8 participants