-
Notifications
You must be signed in to change notification settings - Fork 48
Code and config change: Packet to equinixmetal #1545
Conversation
478161a
to
b89c4d9
Compare
I'd rather use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it make sense to break down the renaming into this many commits. Even though it would be a single, large changeset, it would be more atomic to do those changes in single commit, especially that we also mix some link fixing as part of this PR.
I also think some mentions of Packet are still not renamed, as git grep -i packet
gives plenty of legitimate results, like:
.golangci.yml: - packet
.golangci.yml: - packet_fluo
...
assets/charts/control-plane/packet-ccm/Chart.yaml:name: packet-ccm
...
docs/configuration-reference/components/cluster-autoscaler.md: provider = "packet"
...
scripts/find-updates.sh:get_latest_release packethost/terraform-provider-packet
...
test/calico/metadata_access_test.go: case testutil.PlatformPacket, testutil.PlatformPacketARM:
...
I'm fine if some of those should be considered out of scope for this PR, but still it would be nice to remain as consistent as possible.
Perhaps this one requires a migration or a feature request to Flatcar?
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/cl/controller.yaml.tmpl: ExecStart=/bin/sh -c 'echo "ETCD_LISTEN_CLIENT_URLS=https://$COREOS_PACKET_IPV4_PRIVATE_0:2379" > /etc/kubernetes/etcd.config && echo "ETCD_LISTEN_PEER_URLS=https://$COREOS_PACKET_IPV4_PRIVATE_0:2380" >> /etc/kubernetes/etcd.config && echo "ETCD_LISTEN_METRICS_URLS=http://$COREOS_PACKET_IPV4_PRIVATE_0:2381" >> /etc/kubernetes/etcd.config'
But overall looks good. Nice work @ipochi, a lot of effort into making this finally happen!
Migration Guide used sed so thats what I went with. |
Hmm, can we at least try to see if |
@invidian I've tried to make sure that after every commit I run Making it a single large commit would be a lot to deal with review and maintain sanity. However what we can do is that we can review as multiple commits and then when I get LGTM I can squash all commits into one. |
@invidian |
b89c4d9
to
69eceee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but there is still some code referencing Packet from what I see. Is it going to be addressed in the following PR?
I couldn't get it to work for all the resources, example:
This makes use use
Upstream repo does not have any changes w.r.t renaming from Packet -> Equinix Metal. We have an optional
which by the looks of it is hacky and ugly. I am of the opinion that we should leave the cluster-autoscaler as is. Instead focus on updating to the latest helm chart. The latest release has a commit that fixes some API errors (no sure if it fixes everything) but it would be good to get the latest chart. |
|
69eceee
to
ebe6944
Compare
Sounds good to me as well. Thanks for investigating @ipochi 👍 |
I still see a lot of references to Packet, which can definitely be changed to Equnix Metal: $ git grep -i packet | grep -v cluster-autoscaler | grep -v vendor | grep -v formerly | grep -v packethost | grep -v packet.ipxe | grep -v PACKET_AUTH | grep -v prometheus-operator | grep -v calico | grep -v terraform-provider-packet | grep -v scripts/update | grep -v CHANGELOG.md | grep -v COREOS_PACKET | grep -i packet
README.md:* [Packet quickstart](https://kinvolk.io/docs/lokomotive/latest/quickstarts/packet)
README.md:* [Kubernetes storage with Rook Ceph on Packet cloud](https://kinvolk.io/docs/lokomotive/latest/how-to-guides/rook-ceph-storage)
README.md:* [Setting up an HTTP ingress controller on Lokomotive with MetalLB and Contour on Packet](https://kinvolk.io/docs/lokomotive/latest/how-to-guides/ingress-with-contour-metallb)
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/bootkube.tf: # Select private Packet NIC by using the can-reach Calico autodetection option with the first
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/bootkube.tf: # Block access to Packet metadata service.
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/bootkube.tf: # https://www.packet.com/developers/docs/servers/key-features/metadata/
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/bootkube.tf: # metadata.packet.net should always resolve to 192.80.8.124.
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/cl/controller.yaml.tmpl: # Use 10.0.0.0/8 as this is Packet private network CIDR.
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/controllers.tf: // With newer Packet provider, changing userdata causes re-creation of the device,
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/controllers.tf: flatcar_linux_oem = "packet"
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/controllers.tf: platform = "packet"
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/workers/cl/worker.yaml.tmpl: # Use 10.0.0.0/8 as this is Packet private network CIDR.
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/workers/workers.tf: // With newer Packet provider, changing userdata causes re-creation of the device,
assets/terraform-modules/equinixmetal/flatcar-linux/kubernetes/workers/workers.tf: flatcar_linux_oem = "packet"
... |
1838203
to
7ebaed2
Compare
The following things show Not sure if any of them needs to change or the potential impact of breakage in CI. |
7ebaed2
to
7e50b66
Compare
@ipochi I think the following can still be changed, though they require changes to CI configuration as well. I think all can be rolled out gracefully though.
|
These would require changes in the ci repo as well right ? |
Yes, but I'm fine with doing this changes in the lockstep. Adding support for new names here, then changing CI, then removing the support from here. |
Updated all the necessary references, please review. |
Finally I have the CI green for Packet in a separate pipeline. The issue was related to CCM on two folds.
This patch fixes the issue: kinvolk/metallb@86016b7 It is a workaround and ideally the fix should be in CCM. Issue: kubernetes-sigs/cloud-provider-equinix-metal#199
More details on the issue: kubernetes-sigs/cloud-provider-equinix-metal#200 |
bd44933
to
cf58666
Compare
CI is Green for Equinix Metal and Equinix Metal ARM in separate pipelines Once this is merged and its corresponding CI PR https://github.com/kinvolk/lokomotive-ci/pull/196 , we would need to pause the pipelines, rename them and unpause them. |
56cd24a
to
e4a12d3
Compare
Why can't we make it pass on existing pipelines? |
Need to merge https://github.com/kinvolk/lokomotive-ci/pull/196 first, then unpause and rename and updates pipelines. |
This won't be existing pipelines anymore. |
Let's just merge this and switch the CIs later if that's easier. It's not that big of a deal if we're not in a perfect CI state all the time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
e4a12d3
to
9314031
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If CI is passing somewhere and Ingress remains working, LGTM 👍
9314031
to
9736966
Compare
This commit migrates terraform related resources from Packet to Equinix Metal. It does so by: - replacing the Terraform provider `packethost/packet` with `equinix/metal`. - replacing the prefix `packet_` in `assets/terraform/packet/*` *.tf files with the prefix `metal_`. - Updates the module name from `module.packet` to `module.equinixmetal`, renames the directory `assets/terraform-modules/packet` to `assets/terraform-modules/equinixmetal` - change description in Terraform variables to Equinix Metal - rename code references from Packet to EquinixMetal - test: rename file and dir to equinixmetal - build tags: rename from packet to equinixmetal - docs: update packet to equinixmetal in configuration - ci: change packet to equinixmetal in cluster config - scripts: replace Packet with Equinix Metal - test: Rename PlatformPacket to PlatformEquinixMetal - update env vars referencing Packet to EM - ci: packet to equinixmetal Signed-off-by: Imran Pochi <[email protected]>
This commit removes the Packet Cloud Controller Manager a.k.a packet-ccm as part of the migration to Equinix Metal. In its place, `cloud-provider-equinix-metal` is installed. For new Lokomotive clusters there won't be any difference because well the are fresh clusters. However, for existing clusters, users would need to uninstall the `packet-ccm` helm release and then proceed with cluster upgrade using `lokoctl cluster apply ...`. Note: During upgrade the user must pass the flag `--skip-pre-update-health-check`. Execution steps for existing clusters: 1. Uninstall `packet-ccm` helm release ``` helm uninstall packet-ccm --namespace kube-system ``` 2. Upgrade Lokomotive cluster to the latest release Note: Do not forget the `--skip-pre-update-health-check` flag ``` lokoctl cluster apply --skip-components --skip-pre-update-health-check ``` Signed-off-by: Imran Pochi <[email protected]>
This image includes the PR: kubernetes-sigs/cloud-provider-equinix-metal#201 Signed-off-by: Imran Pochi <[email protected]>
9736966
to
1136eb2
Compare
This commit removes the labels passed to the MetalLB ConfigMap, from nowonwards we only pass annotations. Also removes the custom values such as `metallb.lokomotive.io/<string>` for my-asn, peer-address, peer-asn and source-address. Since the move from Packet CCM to Cloud Provider Equinix Metal, we have removed such custom annotation values from CCM and hence, these values are removed from MetalLB as well. Also updated the MetalLB image to include a fix for supporting multiple peers. This image is based on top of peer autodiscovery support (metallb/metallb#593) and additionally contains the below commit on top. kinvolk/metallb@86016b7 Signed-off-by: Imran Pochi <[email protected]>
This PR builds on top of #1537 by adding code and configuration changes to migrate from Packet to Equinix Metal.
Addresses: #1546
The following changes will take affect after this PR is merged.
packethost/packet
is replaced withequinix/metal
.cluster "packet" {}
tocluster "equinixmetal" {}
Migration steps:
Backup existing clusters state file
Uninstall
packet-ccm
helm releaseReplace the
packethost/packet
Terraform provider withequinix/metal
:packet_
tometal_
in the terraform state file.module.packet
tomodule.equinixmetal
in the terraform state filecluster
module in cluster configuration file(lokocfg) frompacket
toequinixmetal
, example:Note: Do not forget the
--skip-pre-update-health-check
flagTerraform plan should reflect the following changes: Lokomotive control plane and components update, removal of
packet-ccm
, update of thelokoctl-version
and the addition ofcloud-provider-equinix-metal
.Upgrade Lokomotive cluster to the latest release by proceeding with the cluster apply in step 5 with
yes