Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Prompting user to delete mci when all clusters are removed #173

Merged
merged 1 commit into from
Apr 4, 2018

Conversation

nikhiljindal
Copy link
Contributor

@nikhiljindal nikhiljindal commented Apr 4, 2018

Last PR for #58.

Updating remove-clusters command to require --force to remove the ingress from all existing clusters (as per discussion in #58 (comment)).
Also using --force to continue in case of errors.

cc @G-Harmon


This change is Reviewable

@G-Harmon
Copy link
Contributor

G-Harmon commented Apr 4, 2018

:lgtm:


Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


app/kubemci/pkg/gcp/loadbalancer/loadbalancersyncer_test.go, line 466 at r1 (raw file):

	}
	expectedClusters = []string{"cluster2"}
	expectedIGlinks = []string{igLink}

Can you make sure "make fmt"is clean? (I think the problem we have is that "make fmt" returns 0 even when there are things mis-formatted.)


Comments from Reviewable

@nikhiljindal
Copy link
Contributor Author

Review status: 1 of 2 files reviewed at latest revision, 1 unresolved discussion.


app/kubemci/pkg/gcp/loadbalancer/loadbalancersyncer_test.go, line 466 at r1 (raw file):

Previously, G-Harmon wrote…

Can you make sure "make fmt"is clean? (I think the problem we have is that "make fmt" returns 0 even when there are things mis-formatted.)

Done.


Comments from Reviewable

@nikhiljindal
Copy link
Contributor Author

Thanks for the quick review @G-Harmon
Updated as per comments.
Will merge once tests come back green

@nikhiljindal nikhiljindal merged commit c595d5d into GoogleCloudPlatform:master Apr 4, 2018
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request Apr 6, 2018
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Updating kubemci remove-clusters e2e test to use --force to remove from all clusters

Ref GoogleCloudPlatform/k8s-multicluster-ingress#173

Updating kubemci remove-clusters e2e test to validate that removing all clusters throws an error. It should succeed with --force=true

cc @G-Harmon

```release-note
NONE
```
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants