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

deploy: fix a bug of ceph-csi helm chart rendered duplicate affinities in rbd and cephfs #3751

Merged
merged 1 commit into from
May 22, 2023
Merged

deploy: fix a bug of ceph-csi helm chart rendered duplicate affinities in rbd and cephfs #3751

merged 1 commit into from
May 22, 2023

Conversation

dashjay
Copy link
Contributor

@dashjay dashjay commented Apr 17, 2023

Describe what this PR does

When I try to deploy the ceph-csi-rbd chart with value provisioner.affinity, there will be two affinities field in deployment provisioner.

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.

Fixes: #3750

@mergify mergify bot added component/deployment Helm chart, kubernetes templates and configuration Issues/PRs bug Something isn't working labels Apr 17, 2023
@dashjay dashjay force-pushed the deploy/fix-ceph-csi-rbd-provisioner-affinity branch from f91eaec to 824e483 Compare April 17, 2023 08:02
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Apr 18, 2023

@dashjay Thank you for the PR, please refer to https://github.com/ceph/ceph-csi/blob/devel/docs/development-guide.md#code-contribution-workflow to fix CI issue

@dashjay dashjay force-pushed the deploy/fix-ceph-csi-rbd-provisioner-affinity branch from 824e483 to 11be8bd Compare April 18, 2023 09:35
@dashjay dashjay changed the title fix deploy helm chart ceph-csi-rbd-provisioner dup affinities bug deploy: fix bug of ceph-csi-rbd helm chart rendered duplicate affinities. Apr 18, 2023
@dashjay
Copy link
Contributor Author

dashjay commented Apr 18, 2023

@Madhu-1 I have edited the commit message. This is the first time that I do contribution for open source repo, thank you for your patience.

Madhu-1
Madhu-1 reviewed Apr 27, 2023
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.

@dashjay LGTM, small nit

charts/ceph-csi-rbd/templates/provisioner-deployment.yaml Outdated Show resolved Hide resolved
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Apr 27, 2023

same change is required for cephfs also https://github.com/ceph/ceph-csi/blob/devel/charts/ceph-csi-cephfs/templates/provisioner-deployment.yaml#L36-L50

@dashjay
Copy link
Contributor Author

dashjay commented May 10, 2023

same change is required for cephfs also https://github.com/ceph/ceph-csi/blob/devel/charts/ceph-csi-cephfs/templates/provisioner-deployment.yaml#L36-L50

I will do the same thing for this

@dashjay dashjay force-pushed the deploy/fix-ceph-csi-rbd-provisioner-affinity branch 2 times, most recently from ebc0f0c to cf06a29 Compare May 10, 2023 05:44
@dashjay dashjay changed the title deploy: fix bug of ceph-csi-rbd helm chart rendered duplicate affinities. deploy: fix a bug of ceph-csi helm chart rendered duplicate affinities in rbd and cephfs May 10, 2023
@dashjay
Copy link
Contributor Author

dashjay commented May 10, 2023

@Madhu-1 Thank you for reviewing my codes, I did the same thing for cephfs, maybe it need to be review again

Madhu-1
Madhu-1 previously approved these changes May 10, 2023
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.

LGTM

@dashjay dashjay force-pushed the deploy/fix-ceph-csi-rbd-provisioner-affinity branch from cf06a29 to 1a84757 Compare May 10, 2023 09:38
@mergify mergify bot dismissed Madhu-1’s stale review May 10, 2023 09:38

Pull request has been modified.

yati1998
yati1998 previously approved these changes May 10, 2023
View reviewed changes
@yati1998 yati1998 requested a review from Madhu-1 May 10, 2023 10:08
@Madhu-1
Copy link
Collaborator

Madhu-1 commented May 10, 2023

@dashjay can you please take care of CI failure https://github.com/ceph/ceph-csi/actions/runs/4935477418/jobs/8822462022?pr=3751

@dashjay dashjay force-pushed the deploy/fix-ceph-csi-rbd-provisioner-affinity branch from 1a84757 to 299b9fb Compare May 10, 2023 13:06
@mergify mergify bot dismissed yati1998’s stale review May 10, 2023 13:06

Pull request has been modified.

@dashjay
Copy link
Contributor Author

dashjay commented May 10, 2023

@Madhu-1 Sorry for disturbing you again, I amend the commit for passing the commitlint.

@Madhu-1 Madhu-1 added the ci/skip/multi-arch-build skip building on multiple architectures label May 10, 2023
@Madhu-1 Madhu-1 requested review from yati1998 and a team May 10, 2023 14:00
@riya-singhal31
Copy link
Contributor

riya-singhal31 commented May 11, 2023 •
edited
Loading

Hey,@yati1998 , can you please review this.

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

/retest ci/centos/k8s-e2e-external-storage/1.24

@ceph-csi-bot
Copy link
Collaborator

@dashjay "ci/centos/k8s-e2e-external-storage/1.24" test failed. Logs are available at location for debugging

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented May 19, 2023

requeue

❌ This pull request head commit has not been previously disembarked from queue.

@nixpanic
Copy link
Member

@Mergifyio rebase

@dashjay @nixpanic
deploy: fix bug of ceph-csi-rbd helm chart
d396e77 
fix bug that make provisioner get dup affinities
when deploy helm chart ceph-csi-rbd and ceph-csi-cephfs.

Signed-off-by: DashJay <45532257+dashjay@users.noreply.github.com>
@mergify
Copy link
Contributor

mergify bot commented May 19, 2023

rebase

✅ Branch has been successfully rebased

@nixpanic nixpanic force-pushed the deploy/fix-ceph-csi-rbd-provisioner-affinity branch from f18d766 to d396e77 Compare May 19, 2023 14:31
@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label May 19, 2023
@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

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

@github-actions
Copy link

/test ci/centos/upgrade-tests-cephfs

@github-actions
Copy link

/test ci/centos/upgrade-tests-rbd

@github-actions github-actions bot removed the ok-to-test Label to trigger E2E tests label May 19, 2023
@dashjay
Copy link
Contributor Author

dashjay commented May 22, 2023

I'm trying to handle this lint error

@mergify mergify bot merged commit 9df4634 into ceph:devel May 22, 2023
@Rakshith-R
Copy link
Contributor

I'm trying to handle this lint error

It seems to be an intermittent error, I restarted the action & it passed.
Thanks for the contribution @dashjay :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@riya-singhal31 riya-singhal31 riya-singhal31 approved these changes

@yati1998 yati1998 Awaiting requested review from yati1998

Assignees
No one assigned
Labels
bug Something isn't working ci/retry/e2e Label to retry e2e retesting on approved PR's 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
7 participants
@dashjay @Madhu-1 @riya-singhal31 @nixpanic @ceph-csi-bot @Rakshith-R @yati1998