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

feat(kumactl) merge install ingress into install control-plane #1038

Merged
merged 11 commits into from
Sep 29, 2020

Conversation

austince
Copy link
Contributor

@austince austince commented Sep 22, 2020

Summary

Merge and remove the kumactl install ingress command into the kumactl install control-plane

Full changelog

  • Implement merging the current install ingress flags into the install control-plane command, under the --ingress prefix
  • Removes the install ingress command
  • Fix mapping of the repository args

Issues resolved

Fix #1020

Documentation

@austince austince requested a review from a team as a code owner September 22, 2020 15:57
Signed-off-by: austin ce <austin.cawley@gmail.com>
Signed-off-by: austin ce <austin.cawley@gmail.com>
@austince austince force-pushed the feat/kumactl/merge-install-ingress branch from 6399494 to d1bf994 Compare September 22, 2020 16:00
@austince
Copy link
Contributor Author

One question I had during implementation, which applies to both this command and the Helm chart, is: how would a user deploy multiple ingresses to different meshes? Would they need multiple control planes? This seems like a reason to perhaps split the Helm chart or allow multiple ingresses to be installed at once.

@jakubdyszkiewicz
Copy link
Contributor

If you are talking about Kuma Ingress for cross-cluster communication then it works regardless of the mesh, so you can have many Meshes in your CP and one Ingress.

@austince
Copy link
Contributor Author

If you are talking about Kuma Ingress for cross-cluster communication then it works regardless of the mesh, so you can have many Meshes in your CP and one Ingress.

Got it, thanks. The current Ingress Deployment in the chart specifies a kuma.io/mesh: default-or-user-defined annotation for the pods -- is this still the same functionality?

Signed-off-by: austin ce <austin.cawley@gmail.com>
@austince austince force-pushed the feat/kumactl/merge-install-ingress branch from d1bf994 to b353b76 Compare September 22, 2020 16:16
Signed-off-by: austin ce <austin.cawley@gmail.com>
Signed-off-by: austin ce <austin.cawley@gmail.com>
Signed-off-by: austin ce <austin.cawley@gmail.com>
@lobkovilya
Copy link
Contributor

Hey @austince! Ideally, we would like to get rid of Mesh field when it comes to Ingress resources. But now we don't have such concept as a non-mesh scoped resources, so all resources right now belong to some mesh. It's actually the same issue that we have for Zones - #1023.

Answering your question - yes, it is the same functionality even if the user provides some specific value for kuma.io/mesh. Kuma CP anyway looks through all meshes to generate Ingress resource.

@austince
Copy link
Contributor Author

Cool, thanks for the explanation Ilya!

@austince
Copy link
Contributor Author

Looks like just a failing e2e test, but I think ready for review :)

@jakubdyszkiewicz
Copy link
Contributor

I'm fixing the flakiness of e2e in a separate PR.

Nikolay Nikolaev added 5 commits September 28, 2020 12:56
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
@nickolaev nickolaev merged commit 8dbff46 into kumahq:master Sep 29, 2020
@austince austince deleted the feat/kumactl/merge-install-ingress branch September 29, 2020 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge kumactl install ingress in the main kumactl install control-plane
4 participants