Add Packet CCM to Packet platform by knrt10 · Pull Request #1155 · kinvolk/lokomotive

knrt10 · 2020-11-03T09:19:49Z

See commits for more information

Closes #548
Closes #1126

Signed-off-by: knrt10 kautilya@kinvolk.io

assets/terraform-modules/packet/flatcar-linux/kubernetes/bootkube.tf

assets/terraform-modules/bootkube/assets.tf

assets/terraform-modules/packet/flatcar-linux/kubernetes/bootkube.tf

pkg/platform/packet/packet.go

assets/charts/control-plane/kubelet/templates/kubelet-ds.yaml

assets/terraform-modules/bootkube/assets.tf

assets/terraform-modules/bootkube/conditional.tf

test/components/packet_ccm_test.go

docs/configuration-reference/platforms/packet.md

pkg/platform/platform.go

invidian

Looking better.

pkg/platform/packet/packet_test.go

assets/terraform-modules/packet/flatcar-linux/kubernetes/variables.tf

assets/terraform-modules/bootkube/assets.tf

assets/terraform-modules/packet/flatcar-linux/kubernetes/variables.tf

assets/terraform-modules/packet/flatcar-linux/kubernetes/controllers.tf

assets/terraform-modules/packet/flatcar-linux/kubernetes/workers/variables.tf

assets/terraform-modules/packet/flatcar-linux/kubernetes/workers/cl/worker.yaml.tmpl

assets/terraform-modules/packet/flatcar-linux/kubernetes/cl/controller.yaml.tmpl

assets/charts/control-plane/packet-ccm/templates/deployment.yaml

invidian

Just some nits for the most recent changes. Do keep in mind that previous review comments which has not been resolved are still relevant.

assets/terraform-modules/bootkube/assets.tf

pkg/platform/packet/template.go

assets/terraform-modules/packet/flatcar-linux/kubernetes/cl/controller.yaml.tmpl

knrt10 · 2020-11-06T12:13:06Z

Latest Update: CCM works fine. It is currently deployed as a Deployment and run via bootstrapping.

CCM uses --cloud-provider=external for all kubelet which will taint all nodes in a cluster with node.cloudprovider.kubernetes.io/uninitialized. The CCM itself will untaint those nodes when it initializes them. Any pod that does not tolerate that taint will be unscheduled until the CCM is running.

We are not deploying upstream MetalLB as deploying loadbalancer with CCM is optional. When the user applies Lokomotive's MetalLB component CCM will automatically pick it up. Also the previous issue that was failing CCM on re-applying is fixed with upstrem PR. CCM is tested with latest docker image packethost/packet-ccm:f669a86 and it worked fine.

We should wait for their latest release, which will address all the bugs. We have also opened an issue upstream for creating an managing Helm chart for CCM.

Previous review comments all are almost addressed. Would recommend for reviews again.

invidian · 2020-11-09T10:16:03Z

I'll pick it up and continue working on it.

assets/charts/control-plane/packet-ccm/templates/deployment.yaml

iaguis · 2020-12-02T11:39:12Z

This needs a rebase.

docs/configuration-reference/platforms/packet.md

assets/charts/control-plane/packet-ccm/templates/deployment.yaml

test/platform/packet/packet_test.go

assets/terraform-modules/packet/flatcar-linux/kubernetes/workers/cl/worker.yaml.tmpl

Running CCM on Packet requires passing Packet API key to the deployment, which is currently done trough Terraform, so we can no longer just let Packet Terraform provider read this environment variable for it's operation, but we must pass it to Terraform module. This commit ensures, that if user did not configure auth token, value from environment variable will be still written into Terraform code and passed to the controller module. Signed-off-by: knrt10 <kautilya@kinvolk.io>

Using CCM on Kubernetes requires --cloud-provider to be set to "external". This commit enables this customization. Default value remains empty string, so no flag will be set for compatibility with other platforms. Refs #548 Signed-off-by: knrt10 <kautilya@kinvolk.io>

This commit add packet-ccm Helm chart based on manifests from https://github.com/packethost/packet-ccm/tree/master/deploy/template, but with Lokomotive-specific modifications like using image with BGP Node Selector support, enabled leader election for HA, removed unnecessary RBAC grants etc. This chart will be used to enable CCM on Packet platform. Refs #548 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

This label will be used by Packet CCM to select which nodes should have BGP session created. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

There is no need to manually label the nodes now for tests, as Lokomotive on Packet ships those labels automatically for all worker nodes. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

So we don't hit a regression when we move the responsibility of enabling BGP for nodes from Terraform to Packet CCM. Refs #548 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

Which are set by Packet CCM right now. Refs #548 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

This commit enables CCM for Packet platform to allow dropping hacky way of labeling Node objects on Packet via Ignition script for MetalLB autodiscovery to work, which we are doing right now. Closes #548 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

iaguis

lgtm

Addressed.

knrt10 requested review from invidian, ipochi and surajssd November 3, 2020 09:19

knrt10 changed the title ~~Add packet ccm to Packet platforms~~ Add packet ccm to Packet platform Nov 3, 2020

knrt10 force-pushed the knrt10/packet-ccm branch 2 times, most recently from 059ac6b to 1cd889b Compare November 3, 2020 09:25

surajssd previously requested changes Nov 3, 2020

View reviewed changes

assets/terraform-modules/packet/flatcar-linux/kubernetes/bootkube.tf Outdated Show resolved Hide resolved

assets/terraform-modules/bootkube/assets.tf Outdated Show resolved Hide resolved

knrt10 force-pushed the knrt10/packet-ccm branch 5 times, most recently from 906894d to 4a7eb43 Compare November 3, 2020 14:18

knrt10 requested a review from surajssd November 3, 2020 14:18

knrt10 force-pushed the knrt10/packet-ccm branch 2 times, most recently from a8573fa to c061070 Compare November 3, 2020 15:02

invidian suggested changes Nov 3, 2020

View reviewed changes

knrt10 force-pushed the knrt10/packet-ccm branch from c061070 to 0049640 Compare November 4, 2020 14:10

invidian suggested changes Nov 4, 2020

View reviewed changes

knrt10 force-pushed the knrt10/packet-ccm branch from 0049640 to a8ac0c2 Compare November 5, 2020 11:42

knrt10 requested a review from invidian November 5, 2020 11:53

knrt10 force-pushed the knrt10/packet-ccm branch from a8ac0c2 to 37c9299 Compare November 5, 2020 14:31

invidian suggested changes Nov 5, 2020

View reviewed changes

assets/terraform-modules/bootkube/assets.tf Show resolved Hide resolved

pkg/platform/packet/template.go Show resolved Hide resolved

assets/terraform-modules/packet/flatcar-linux/kubernetes/cl/controller.yaml.tmpl Show resolved Hide resolved

knrt10 force-pushed the knrt10/packet-ccm branch 2 times, most recently from 9b662f4 to 8ff429b Compare November 6, 2020 06:15

knrt10 requested a review from invidian November 6, 2020 06:22

knrt10 force-pushed the knrt10/packet-ccm branch from 8ff429b to e8969e9 Compare November 6, 2020 06:31

invidian force-pushed the knrt10/packet-ccm branch 2 times, most recently from b6c6797 to c17e8d7 Compare November 10, 2020 17:44

knrt10 commented Nov 20, 2020

View reviewed changes

assets/charts/control-plane/packet-ccm/templates/deployment.yaml Show resolved Hide resolved

invidian force-pushed the knrt10/packet-ccm branch 2 times, most recently from d679af6 to 7fe4480 Compare November 20, 2020 13:34

knrt10 force-pushed the knrt10/packet-ccm branch 3 times, most recently from 3f58bef to 0306aa7 Compare November 23, 2020 02:56

invidian force-pushed the knrt10/packet-ccm branch from 0306aa7 to 496032d Compare November 23, 2020 12:14

invidian previously approved these changes Nov 23, 2020

View reviewed changes

invidian requested a review from iaguis November 23, 2020 12:34

invidian dismissed their stale review via 1cee197 December 2, 2020 12:32

invidian force-pushed the knrt10/packet-ccm branch 2 times, most recently from 1cee197 to c7022ef Compare December 2, 2020 12:43

iaguis suggested changes Dec 2, 2020

View reviewed changes

knrt10 added 2 commits December 3, 2020 10:04

invidian force-pushed the knrt10/packet-ccm branch from c7022ef to 67d5710 Compare December 3, 2020 09:37

invidian added 6 commits December 3, 2020 11:04

packet: add bgp-enabled label to workers

28702e7

This label will be used by Packet CCM to select which nodes should have BGP session created. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

Use new bgp-enabled label while testing BGP

e8612a8

There is no need to manually label the nodes now for tests, as Lokomotive on Packet ships those labels automatically for all worker nodes. Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

test/platform/packet: add test for enabling BGP

ed52211

So we don't hit a regression when we move the responsibility of enabling BGP for nodes from Terraform to Packet CCM. Refs #548 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

pkg/components/metallb: read peer config also from annotations

a7217c4

Which are set by Packet CCM right now. Refs #548 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

packet: enable CCM

02b1ef6

This commit enables CCM for Packet platform to allow dropping hacky way of labeling Node objects on Packet via Ignition script for MetalLB autodiscovery to work, which we are doing right now. Closes #548 Signed-off-by: Mateusz Gozdek <mateusz@kinvolk.io>

invidian force-pushed the knrt10/packet-ccm branch from 67d5710 to 02b1ef6 Compare December 3, 2020 10:04

iaguis approved these changes Dec 3, 2020

View reviewed changes

invidian merged commit 16c5ff8 into master Dec 3, 2020

invidian deleted the knrt10/packet-ccm branch December 3, 2020 11:30

invidian mentioned this pull request Dec 7, 2020

Release v0.6.0 #1064

Closed

surajssd mentioned this pull request Dec 23, 2020

Direct upgrade from 0.5.0 to latest master for MetalLB is broken #1283

Closed

Conversation

knrt10 commented Nov 3, 2020 • edited by invidian Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

invidian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

invidian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

knrt10 commented Nov 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

invidian commented Nov 9, 2020

Uh oh!

Uh oh!

iaguis commented Dec 2, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iaguis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

knrt10 commented Nov 3, 2020 •

edited by invidian

Loading

knrt10 commented Nov 6, 2020 •

edited

Loading