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

Remove stale volumes if endpoint/svc creation fails. #35285

Merged
merged 1 commit into from Oct 31, 2016
Merged

Remove stale volumes if endpoint/svc creation fails. #35285

merged 1 commit into from Oct 31, 2016

Conversation

humblec
Copy link
Contributor

@humblec humblec commented Oct 21, 2016 •
edited by jessfraz 

Remove stale volumes if endpoint/svc creation fails.

Signed-off-by: Humble Chirammal hchiramm@redhat.com

This change is Reviewable

@humblec
Remove stale volumes if endpoint/svc creation fails.
9026347 
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
@k8s-ci-robot
Copy link
Contributor

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "@k8s-bot ok to test" on its own line.

Regular contributors should join the org to skip this step.

@humblec
Copy link
Contributor Author

humblec commented Oct 21, 2016

@rootfs @jsafrane PTAL.

@k8s-github-robot k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Oct 21, 2016
@jsafrane
Copy link
Member

@k8s-bot ok to test

@humblec
Copy link
Contributor Author

humblec commented Oct 21, 2016

@jsafrane can you set an IGNORE here to retrigger the tests?

@jsafrane
Copy link
Member

@k8s-bot test this issue: #IGNORE

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit 9026347. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@humblec
Copy link
Contributor Author

humblec commented Oct 21, 2016

@rootfs can you please review/merge this change?

rootfs
rootfs reviewed Oct 21, 2016
View reviewed changes
pkg/volume/glusterfs/glusterfs.go
@@ -550,6 +550,10 @@ func (p *glusterfsVolumeProvisioner) CreateVolume() (r *api.GlusterfsVolumeSourc
endpoint, service, err := p.createEndpointService(epNamespace, epServiceName, dynamicHostIps, p.options.PVC.Name)
if err != nil {
glog.Errorf("glusterfs: failed to create endpoint/service")
err = cli.VolumeDelete(volume.Id)
if err != nil {
glog.Errorf("glusterfs: error when deleting the volume :%v , manual deletion required", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

why volumeDelete will get stale endpoint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rootfs no.. its other way around, on volume creation code path, if it created volume and then while trying to create an endpoint if it fails, previously created volume will exist in the backend. When next provisioning request comes it will create one more volume .. so on. This patch delete the volume it created if there is a failure in endpoint/svc creation.

@rootfs
Copy link
Contributor

rootfs commented Oct 25, 2016

lgtm @jsafrane ?

@jsafrane jsafrane added sig/storage Categorizes an issue or PR as relevant to SIG Storage. team/cluster release-note-none Denotes a PR that doesn't merit a release note. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed release-note-label-needed labels Oct 26, 2016
@humblec
Copy link
Contributor Author

humblec commented Oct 29, 2016

This PR is almost 3 days in the submit queue, still not merged. Not sure whats wrong ?

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit 9026347. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@jsafrane
Copy link
Member

@k8s-bot cvm gce e2e test this
[build timed out]

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 1064927 into kubernetes:master Oct 31, 2016
@childsb childsb mentioned this pull request Nov 2, 2016
Merged
@soltysh
Copy link
Contributor

soltysh commented Nov 2, 2016

@humblec should that be cherry-picked into 1.4 ?

@humblec
Copy link
Contributor Author

humblec commented Nov 3, 2016

@soltysh Thanks, yeah, really good if we can !

@soltysh soltysh added this to the v1.4 milestone Nov 3, 2016
@soltysh
Copy link
Contributor

soltysh commented Nov 3, 2016

@jessfraz another 1.4 cherry-pick candidate

@humblec
Copy link
Contributor Author

humblec commented Nov 4, 2016

Thanks a lot @soltysh and @jessfraz !

@jessfraz jessfraz added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Nov 8, 2016
@jessfraz
Copy link
Contributor

jessfraz commented Nov 8, 2016

cool I'll do all these as a group today

@jessfraz jessfraz mentioned this pull request Nov 9, 2016
Merged
@jessfraz
Copy link
Contributor

jessfraz commented Nov 9, 2016

cherry-picked in #36510

@jessfraz jessfraz added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 9, 2016
jessfraz added a commit that referenced this pull request Nov 9, 2016
@jessfraz
Merge pull request #36510 from jessfraz/automated-cherry-pick-of-#34375-
d72ad89 
#35285-#35420-#35608-#35919-#36161-origin-release-1.4

Automated cherry pick of #34375 #35285 #35420 #35608 #35919 #36161
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
@jessfraz
Merge pull request kubernetes#36510 from jessfraz/automated-cherry-pi…
875d3fe 
…ck-of-#34375-kubernetes#35285-kubernetes#35420-kubernetes#35608-kubernetes#35919-kubernetes#36161-origin-release-1.4

Automated cherry pick of kubernetes#34375 kubernetes#35285 kubernetes#35420 kubernetes#35608 kubernetes#35919 kubernetes#36161
@chentao1596 chentao1596 mentioned this pull request Dec 5, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rootfs rootfs rootfs left review comments

Assignees

@rootfs rootfs

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@humblec @k8s-ci-robot @jsafrane @rootfs @k8s-github-robot @soltysh @jessfraz @googlebot