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

Fix leader election in deployment #428

Closed
wants to merge 3 commits into from
Closed

Conversation

Madhu-1
Copy link
Collaborator

@Madhu-1 Madhu-1 commented Jun 13, 2019

Describe what this PR does

  • Enable leader election for provisioner
  • Update e2e to create and delete multiple PVC and app
  • Added backend image validation for rbd

Is there anything that requires special attention

Related issues

Fixes: #426

Madhu-1 added 2 commits June 13, 2019 13:56
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
pvc and app

added backend image validation for rbd

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jun 13, 2019

@humblec @ShyamsundarR PTAL

@Madhu-1 Madhu-1 added the release-1.1.0 Track release 1.1 items label Jun 13, 2019
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jun 13, 2019

Ci is failing with

The command "curl -sf "https://install.goreleaser.com/github.com/golangci/golangci-lint.sh" | bash -s -- -b $GOPATH/bin "${GOLANGCI_VERSION}"" failed and exited with 1 during .

this PR haven't changes anything related to this

@@ -30,7 +30,8 @@ spec:
image: "{{ .Values.provisioner.image.repository }}:{{ .Values.provisioner.image.tag }}"
args:
- "--csi-address=$(ADDRESS)"
- "--leader-election-type=leases"
- "--enable-leader-election=true"
- "--leader-election-type=endpoints"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we moving away from leases? Kubernetes CSI seems to be suggesting that this option itself would be deprecated in the future as the election mechanism would default to leases, so why are we starting with endpoints and what are the repercussions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ShyamsundarR external-provisoner is failing with leases kubernetes-csi/external-provisioner#291

Copy link
Contributor

Choose a reason for hiding this comment

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

endpoints seem to have a request flooding problem, as noted here. In consideration of this should we even proceed with endpoints or move back to Stateful sets, or should we wait for a fix/ack for the Kubernetes issue?

Copy link
Collaborator Author

@Madhu-1 Madhu-1 Jun 13, 2019

Choose a reason for hiding this comment

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

seems like leases works with kube 1.14+ version. do we need to switch back to statefulset to support older kubernetes version or will use endpoint as lease?

one question needs to answer here is how much systems will be affected with throttling enabled due to the endpoint?

Copy link
Collaborator Author

@Madhu-1 Madhu-1 Jun 13, 2019

Choose a reason for hiding this comment

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

CI passed with lease and kube 1.14.2 https://travis-ci.org/ceph/ceph-csi/jobs/545169705

Copy link
Contributor

Choose a reason for hiding this comment

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

seems like leases works with kube 1.14+ version. do we need to switch back to statefulset to support older kubernetes version or will use endpoint as lease?

I think it is too early to drop v1.13 support, so either have 2 deployments (one for each version), or switch back to Stateful sets (not optimal for future requirements of upgrade etc.).

one question needs to answer here is how much systems will be affected with throttling enabled due to the endpoint?

This issue and its related discussion around 100PVCs seems to suggest this could happen at lower threasholds, I would hence be wary of using the endpoint leadership election mechanism till we know better.

I would say have different deployments for different Kubernetes versions, and hence use leases only for v1.14 and Stateful sets for v1.13 is a solution that we should go for.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
@ShyamsundarR
Copy link
Contributor

Ci is failing with

The command "curl -sf "https://install.goreleaser.com/github.com/golangci/golangci-lint.sh" | bash -s -- -b $GOPATH/bin "${GOLANGCI_VERSION}"" failed and exited with 1 during .

this PR haven't changes anything related to this

Other PRs are also failing here: see #427 in travis job https://travis-ci.org/ceph/ceph-csi/jobs/545156584

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jun 13, 2019

Ci is failing with

The command "curl -sf "https://install.goreleaser.com/github.com/golangci/golangci-lint.sh" | bash -s -- -b $GOPATH/bin "${GOLANGCI_VERSION}"" failed and exited with 1 during .

this PR haven't changes anything related to this

Other PRs are also failing here: see #427 in travis job https://travis-ci.org/ceph/ceph-csi/jobs/545156584

Fixed here #429

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jun 14, 2019

Need to enable leader election in this deploment

@Madhu-1 Madhu-1 added the DNM DO NOT MERGE label Jun 14, 2019
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jun 14, 2019

sent a PR #432 to revert back to statefulset

will revisit PR once we sort out things to use deployment

@humblec humblec removed the release-1.1.0 Track release 1.1 items label Jul 11, 2019
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Jul 25, 2019

will be fixed by #497
closing

@Madhu-1 Madhu-1 closed this Jul 25, 2019
Nikhil-Ladha pushed a commit to Nikhil-Ladha/ceph-csi that referenced this pull request Dec 9, 2024
Syncing latest changes from upstream devel for ceph-csi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM DO NOT MERGE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rbd creating multiple images in the backend for single PVC
3 participants