search

GoogleCloudPlatform / k8s-multicluster-ingress

kubemci: Command line tool to configure L7 load balancers using multiple kubernetes clusters
Apache License 2.0
376 stars 68 forks source link

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

Closed nikhiljindal closed 6 years ago

nikhiljindal commented 6 years ago

Last PR for https://github.com/GoogleCloudPlatform/k8s-multicluster-ingress/issues/58.

Updating remove-clusters command to require --force to remove the ingress from all existing clusters (as per discussion in https://github.com/GoogleCloudPlatform/k8s-multicluster-ingress/issues/58#issuecomment-369081019). Also using --force to continue in case of errors.

cc @G-Harmon

This change is Reviewable

G-Harmon commented 6 years ago
: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 commented 6 years ago

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 commented 6 years ago

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