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

ingress controller should only manage instance groups for multicluster ingress #182

Closed
nikhiljindal opened this issue Mar 29, 2018 · 2 comments · Fixed by #183
Closed

ingress controller should only manage instance groups for multicluster ingress #182

nikhiljindal opened this issue Mar 29, 2018 · 2 comments · Fixed by #183

Comments

@nikhiljindal
Copy link
Contributor

Forked from GoogleCloudPlatform/k8s-multicluster-ingress#131

ingress-gce controller is configuring the whole load balancer including creating url map, target proxy, etc for multicluster ingresses.
This is a regression. It should only manage instance groups.

Thanks to @MrHohn for finding the problem.
@nicksardo is working on a fix and I am going to send a PR to update the ingress-gce tests to catch this early.

Filing this issue to track

cc @csbell @G-Harmon @bowei
This was referenced Mar 29, 2018
Closed
Merged
@nicksardo nicksardo mentioned this issue Mar 29, 2018
Merged
@G-Harmon
Copy link
Contributor

nikhil was telling me about this bug today.

k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Mar 31, 2018
Merge pull request #61909 from nikhiljindal/testMCI
1c008c5 
Automatic merge from submit-queue (batch tested with PRs 54997, 61869, 61816, 61909, 60525). 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 multicluster e2e test to ensure that controller only creates instance groups

Ref kubernetes/ingress-gce#182

Updating the multicluster ingress e2e test to also verify that the controller does not create any other resource than instance group.

I was planning to do that using annotations first, but turns out that we have the right check for annotations: https://github.com/kubernetes/ingress-gce/blob/61d574a2c74fbb6f47e1aec0fae385cf2ca97aaf/pkg/controller/controller.go#L317 :)

Am using `length(ListResource) == 0` now but have still kept the annotation check.

cc @nicksardo @MrHohn @G-Harmon

**Release note**:
```release-note
NONE
```
@nikhiljindal
Copy link
Contributor Author

#183 fixed the code and kubernetes/kubernetes#61909 updated the test to ensure that we do not regress.

The test is still failing tough.
Sent kubernetes/kubernetes#61987 to fix that

This was referenced Apr 3, 2018
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix sync of multi-cluster ingress nicksardo/ingress-gce
2 participants
@nikhiljindal @G-Harmon