Add flag to allow userdata to be uncompressed #216

hrak · 2023-01-31T21:07:16Z

Issue #, if available: #215

Description of changes:

This PR adds a boolean flag to the CloudstackMachine spec that allows userdata to be uncompressed. This is required for ignition support (Flatcar f.e.)

Please let me know if this is complete, this is my first PR that touches the api.

Testing performed:

Compiled and build new CAPC container image (which also includes the changes from #214 btw), tested using kind and Cloudstack 4.17 using Flatcar stable 3374.2.3. Was able to spin up a full cluster using 3 control plane nodes and 2 workers.

I used the feature flag export EXP_KUBEADM_BOOTSTRAP_FORMAT_IGNITION=true and the following cluster template:

---
apiVersion: cluster.x-k8s.io/v1beta1
kind: Cluster
metadata:
  name: ${CLUSTER_NAME}
spec:
  clusterNetwork:
    pods:
      cidrBlocks:
        - 192.168.0.0/16
    serviceDomain: cluster.local
  infrastructureRef:
    apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
    kind: CloudStackCluster
    name: ${CLUSTER_NAME}
  controlPlaneRef:
    kind: KubeadmControlPlane
    apiVersion: controlplane.cluster.x-k8s.io/v1beta1
    name: ${CLUSTER_NAME}-control-plane
---
apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
kind: CloudStackCluster
metadata:
  name: ${CLUSTER_NAME}
spec:
  controlPlaneEndpoint:
    host: ${CLUSTER_ENDPOINT_IP}
    port: ${CLUSTER_ENDPOINT_PORT}
  failureDomains:
    - name: ${CLOUDSTACK_FD1_NAME=failure-domain-1}
      acsEndpoint:
        name: ${CLOUDSTACK_FD1_SECRET_NAME=cloudstack-credentials}
        namespace: ${CLOUDSTACK_FD1_SECRET_NAMESPACE=default}
      zone:
        name: ${CLOUDSTACK_ZONE_NAME}
        network:
          name: ${CLOUDSTACK_NETWORK_NAME}
---
apiVersion: controlplane.cluster.x-k8s.io/v1beta1
kind: KubeadmControlPlane
metadata:
  name: "${CLUSTER_NAME}-control-plane"
spec:
  kubeadmConfigSpec:
    format: ignition
    ignition:
      containerLinuxConfig:
        additionalConfig: |
          systemd:
            units:
            - name: kubeadm.service
              enabled: true
              dropins:
              - name: 10-flatcar.conf
                contents: |
                  [Unit]
                  Requires=containerd.service coreos-metadata.service
                  After=containerd.service coreos-metadata.service
                  [Service]
                  EnvironmentFile=/run/metadata/flatcar
            - name: coreos-metadata.service
              dropins:
              - name: 20-clct-provider-override.conf
                contents: |
                  [Service]
                  Environment=COREOS_METADATA_OPT_PROVIDER=--provider=cloudstack-metadata
            - name: coreos-metadata-sshkeys@.service
              enabled: true
              dropins:
              - name: 20-clct-provider-override.conf
                contents: |
                  [Service]
                  Environment=COREOS_METADATA_OPT_PROVIDER=--provider=cloudstack-metadata
    initConfiguration:
      nodeRegistration:
        kubeletExtraArgs:
          provider-id: "cloudstack:///$${COREOS_CLOUDSTACK_INSTANCE_ID}"
        name: $${COREOS_CLOUDSTACK_HOSTNAME}
    joinConfiguration:
      nodeRegistration:
        kubeletExtraArgs:
          provider-id: "cloudstack:///'$${COREOS_CLOUDSTACK_INSTANCE_ID}'"
        name: $${COREOS_CLOUDSTACK_HOSTNAME}
    preKubeadmCommands:
      - export COREOS_CLOUDSTACK_HOSTNAME=$${COREOS_CLOUDSTACK_HOSTNAME%.*}
      - export COREOS_CLOUDSTACK_INSTANCE_ID=$${COREOS_CLOUDSTACK_INSTANCE_ID%.*}
      - envsubst < /etc/kubeadm.yml > /etc/kubeadm.yml.tmp
      - mv /etc/kubeadm.yml.tmp /etc/kubeadm.yml
  machineTemplate:
    infrastructureRef:
      kind: CloudStackMachineTemplate
      apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
      name: "${CLUSTER_NAME}-control-plane"
  replicas: ${CONTROL_PLANE_MACHINE_COUNT}
  version: ${KUBERNETES_VERSION}
---
apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
kind: CloudStackMachineTemplate
metadata:
  name: ${CLUSTER_NAME}-control-plane
spec:
  template:
    spec:
      sshKey: ${CLOUDSTACK_SSH_KEY_NAME}
      uncompressedUserData: true
      offering:
        name: ${CLOUDSTACK_CONTROL_PLANE_MACHINE_OFFERING}
      template:
        name: ${CLOUDSTACK_TEMPLATE_NAME}
---
apiVersion: cluster.x-k8s.io/v1beta1
kind: MachineDeployment
metadata:
  name: "${CLUSTER_NAME}-md-0"
spec:
  clusterName: "${CLUSTER_NAME}"
  replicas: ${WORKER_MACHINE_COUNT}
  selector:
    matchLabels: null
  template:
    spec:
      clusterName: "${CLUSTER_NAME}"
      version: ${KUBERNETES_VERSION}
      bootstrap:
        configRef:
          apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
          kind: KubeadmConfigTemplate
          name: "${CLUSTER_NAME}-md-0"
      infrastructureRef:
        name: "${CLUSTER_NAME}-md-0"
        apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
        kind: CloudStackMachineTemplate
---
apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
kind: CloudStackMachineTemplate
metadata:
  name: ${CLUSTER_NAME}-md-0
spec:
  template:
    spec:
      sshKey: ${CLOUDSTACK_SSH_KEY_NAME}
      uncompressedUserData: true
      offering:
        name: ${CLOUDSTACK_WORKER_MACHINE_OFFERING}
      template:
        name: ${CLOUDSTACK_TEMPLATE_NAME}
---
apiVersion: bootstrap.cluster.x-k8s.io/v1beta1
kind: KubeadmConfigTemplate
metadata:
  name: ${CLUSTER_NAME}-md-0
spec:
  template:
    spec:
      joinConfiguration:
        nodeRegistration:
          kubeletExtraArgs:
            provider-id: "cloudstack:///$${COREOS_CLOUDSTACK_INSTANCE_ID}"
          name: $${COREOS_CLOUDSTACK_HOSTNAME}
      preKubeadmCommands:
        - export COREOS_CLOUDSTACK_HOSTNAME=$${COREOS_CLOUDSTACK_HOSTNAME%.*}
        - export COREOS_CLOUDSTACK_INSTANCE_ID=$${COREOS_CLOUDSTACK_INSTANCE_ID%.*}
        - envsubst < /etc/kubeadm.yml > /etc/kubeadm.yml.tmp
        - mv /etc/kubeadm.yml.tmp /etc/kubeadm.yml
      format: ignition
      ignition:
        containerLinuxConfig:
          additionalConfig: |
            systemd:
              units:
              - name: kubeadm.service
                enabled: true
                dropins:
                - name: 10-flatcar.conf
                  contents: |
                    [Unit]
                    Requires=containerd.service coreos-metadata.service
                    After=containerd.service coreos-metadata.service
                    [Service]
                    EnvironmentFile=/run/metadata/flatcar
              - name: coreos-metadata.service
                dropins:
                - name: 20-clct-provider-override.conf
                  contents: |
                    [Service]
                    Environment=COREOS_METADATA_OPT_PROVIDER=--provider=cloudstack-metadata
              - name: coreos-metadata-sshkeys@.service
                enabled: true
                dropins:
                - name: 20-clct-provider-override.conf
                  contents: |
                    [Service]
                    Environment=COREOS_METADATA_OPT_PROVIDER=--provider=cloudstack-metadata

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

netlify · 2023-01-31T21:07:22Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-cloudstack ready!

Name	Link
🔨 Latest commit	`d84b9f9`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-cloudstack/deploys/63d983079ba38e000857800b
😎 Deploy Preview	https://deploy-preview-216--kubernetes-sigs-cluster-api-cloudstack.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

k8s-ci-robot · 2023-01-31T21:07:22Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hrak
Once this PR has been reviewed and has the lgtm label, please assign rohityadavcloud for approval by writing /assign @rohityadavcloud in a comment. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2023-01-31T21:07:26Z

Hi @hrak. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jweite-amazon · 2023-02-06T14:18:18Z

/ok-to-test

chrisdoherty4 · 2023-02-10T15:14:17Z

api/v1beta2/cloudstackmachine_types.go

+	// cloud-init has built-in support for gzip-compressed user data, ignition does not
+	//
+	// +optional
+	UncompressedUserData *bool `json:"uncompressedUserData,omitempty"`


Does this need to be a pointer? The zero value of a bool will be false and we could add a the default marker to be abundantly clear.

+kubebuilder:default=false +optional

This would simplify some of the implementation code. What do you think?

@chrisdoherty4 The main reason was to be able to check for absolute nil (not false)

if uncompressed != nil && !*uncompressed

but i will gladly adapt if you think that's not necessary.

No worries. Do you have a use-case where we'd take some different action as a result of it being nil vs false?

I used this approach mainly because there is no default value for this setting and i was not sure what it would evaluate to. This is also heavily inspired by cluster-api-provider-aws where they take a similar approach.

The only time we really need a pointer is to differentiate between a user setting the value or not. If we don't need to differentiate those cases then +kubebuilder:default=false should be fine and would make whatever interprets this simpler because they don't need to perform nil pointer checks.

Currently, I don't see a need to differentiate set vs not set. Are you able to clarify?

Hi @chrisdoherty4 i looked a bit further into the reasoning for the pointer. Apparently this is part of the Kubernetes API conventions / best practices (see here.

Other references:

crossplane/crossplane#741
kubernetes-sigs/kubebuilder#2109

That's good to know, thanks for looking into it.

chrisdoherty4

Holistically, this look great!

hrak · 2023-03-08T08:11:01Z

Is there anything i can do to move this forward? Are you OK with my comments?

jweite-amazon · 2023-03-14T14:15:39Z

/retest

k8s-ci-robot · 2023-03-14T14:21:52Z

@hrak: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
capi-provider-cloudstack-presubmit-unit-test	`d84b9f9`	link	true	`/test capi-provider-cloudstack-presubmit-unit-test`
capi-provider-cloudstack-presubmit-build	`d84b9f9`	link	true	`/test capi-provider-cloudstack-presubmit-build`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

hrak · 2023-03-14T16:12:07Z

@jweite-amazon I don't know how to fix these tests. It looks like the Go version of the tests is too old. Can you assist?

edit: Yup, definitely too old:

$ docker run -it public.ecr.aws/r9t0b9f2/capc-builder:latest /bin/sh
WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
# go version
go version go1.17.8 linux/amd64

jweite-amazon · 2023-03-15T20:01:32Z

@hrak, please stand by. Investigation into this is taking place.

chrisdoherty4 · 2023-03-15T20:13:25Z

@jweite-amazon I don't know how to fix these tests. It looks like the Go version of the tests is too old. Can you assist?

edit: Yup, definitely too old:
$ docker run -it public.ecr.aws/r9t0b9f2/capc-builder:latest /bin/sh
WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
# go version
go version go1.17.8 linux/amd64

Looking into fixing this, however it won't be done particularly quickly. These changes seem sound.

/lgtm

jweite-amazon · 2023-03-15T20:42:55Z

/lgtm

Add flag to allow userdata to be uncompressed

d84b9f9

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 31, 2023

k8s-ci-robot requested review from davidjumani and jweite-amazon January 31, 2023 21:07

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 31, 2023

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 31, 2023

hrak mentioned this pull request Jan 31, 2023

Add Cloudstack provider support for Flatcar kubernetes-sigs/image-builder#1064

Closed

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 6, 2023

chrisdoherty4 reviewed Feb 10, 2023

View reviewed changes

k8s-ci-robot assigned jweite-amazon Mar 15, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 15, 2023

jweite-amazon merged commit 98827c6 into kubernetes-sigs:main Mar 15, 2023

hrak mentioned this pull request Mar 17, 2023

Allow user-data compression to be disabled to support ignition #215

Closed

hrak deleted the userdata_uncomp branch March 21, 2023 09:59

chrisdoherty4 mentioned this pull request May 5, 2023

REQUEST: New membership for chrisdoherty4 kubernetes/org#4199

Closed

9 tasks

hrak mentioned this pull request Dec 12, 2023

REQUEST: New membership for hrak kubernetes/org#4623

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add flag to allow userdata to be uncompressed #216

Add flag to allow userdata to be uncompressed #216

hrak commented Jan 31, 2023

netlify bot commented Jan 31, 2023 •

edited

Loading

k8s-ci-robot commented Jan 31, 2023

k8s-ci-robot commented Jan 31, 2023

jweite-amazon commented Feb 6, 2023

chrisdoherty4 Feb 10, 2023 •

edited

Loading

hrak Feb 13, 2023

chrisdoherty4 Feb 13, 2023

hrak Feb 20, 2023

chrisdoherty4 Mar 8, 2023 •

edited

Loading

hrak Mar 14, 2023

chrisdoherty4 Mar 15, 2023

chrisdoherty4 left a comment

hrak commented Mar 8, 2023

jweite-amazon commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

hrak commented Mar 14, 2023 •

edited

Loading

jweite-amazon commented Mar 15, 2023

chrisdoherty4 commented Mar 15, 2023 •

edited

Loading

jweite-amazon commented Mar 15, 2023

Add flag to allow userdata to be uncompressed #216

Add flag to allow userdata to be uncompressed #216

Conversation

hrak commented Jan 31, 2023

netlify bot commented Jan 31, 2023 • edited Loading

✅ Deploy Preview for kubernetes-sigs-cluster-api-cloudstack ready!

k8s-ci-robot commented Jan 31, 2023

k8s-ci-robot commented Jan 31, 2023

jweite-amazon commented Feb 6, 2023

chrisdoherty4 Feb 10, 2023 • edited Loading

Choose a reason for hiding this comment

hrak Feb 13, 2023

Choose a reason for hiding this comment

chrisdoherty4 Feb 13, 2023

Choose a reason for hiding this comment

hrak Feb 20, 2023

Choose a reason for hiding this comment

chrisdoherty4 Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

hrak Mar 14, 2023

Choose a reason for hiding this comment

chrisdoherty4 Mar 15, 2023

Choose a reason for hiding this comment

chrisdoherty4 left a comment

Choose a reason for hiding this comment

hrak commented Mar 8, 2023

jweite-amazon commented Mar 14, 2023

k8s-ci-robot commented Mar 14, 2023

hrak commented Mar 14, 2023 • edited Loading

jweite-amazon commented Mar 15, 2023

chrisdoherty4 commented Mar 15, 2023 • edited Loading

jweite-amazon commented Mar 15, 2023

netlify bot commented Jan 31, 2023 •

edited

Loading

chrisdoherty4 Feb 10, 2023 •

edited

Loading

chrisdoherty4 Mar 8, 2023 •

edited

Loading

hrak commented Mar 14, 2023 •

edited

Loading

chrisdoherty4 commented Mar 15, 2023 •

edited

Loading