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

static-pods: remove conditional compilation #3891

Merged
merged 3 commits into from
Apr 19, 2024

Conversation

jmt-lab
Copy link
Contributor

@jmt-lab jmt-lab commented Apr 11, 2024

Issue number: 3619

Closes #3619

Description of changes:

thar-be-settings + model:

  • Added new option to ConfigurationFile skip-if-empty where if specified and set to true, and the template is resolved to an empty file or file with just whitespace do not write the configuration file to disk.

static-pods:

  • Removed static-pods helper binary
  • Implemented template to directly render the static-pods manifests into a single file only if enabled = true
  • Removed static-pods from os package
  • Added template to all kubernetes-* packages
  • Created migrations

Testing done:

  • Ran cargo +nightly udeps to ensure all unused dependencies have been removed
  • Pass the settings on first boot via userdata. Verify config.
  • cat /etc/os-release record your build ID include commit sha, record this
  • Use apiclient to update relevant settings at runtime. Verify config was updated correctly.
  • Downgrade to older version.
    • verify connectivity
    • cat /etc/os-release to verify lower version
    • verify that migrations have changed the metadata/settings as expected (i.e. they are gone from the datastore, or lists have been changed back)
  • Upgrade to newer version.
    • verify connectivity
    • cat /etc/os-release to verify version
    • verify config file
Test Report

Initial_Build_ID.Output

{
  "os": {
    "arch": "x86_64",
    "build_id": "cf7a4e6e",
    "pretty_name": "Bottlerocket OS 1.20.0 (aws-k8s-1.28)",
    "variant_id": "aws-k8s-1.28",
    "version_id": "1.20.0"
  }
}

Initial_Config.Output

/etc/kubernetes/static-pods-manifest.toml

["one"]
enabled = true
manifest = "YXBpVmVyc2lvbjogdjEKa2luZDogUG9kCm1ldGFkYXRhOgogIG5hbWU6IHN0YXRpYy13ZWIKc3BlYzoKICBjb250YWluZXJzOgogICAgLSBuYW1lOiB3ZWIKICAgICAgaW1hZ2U6IG5naW54"
["two"]
enabled = true
manifest = "YXBpVmVyc2lvbjogdjEKa2luZDogUG9kCm1ldGFkYXRhOgogIG5hbWU6IHN0YXRpYy13ZWIyCnNwZWM6CiAgY29udGFpbmVyczoKICAgIC0gbmFtZTogd2ViCiAgICAgIGltYWdlOiBuZ2lueA=="

configuration-files.static-pods-toml

{
  "configuration-files": {
    "static-pods-toml": {
      "path": "/etc/kubernetes/static-pods-manifest.toml",
      "template-path": "/usr/share/templates/static-pods-toml"
    }
  }
}

services.static-pods

{
  "services": {
    "static-pods": {
      "configuration-files": [
        "static-pods-toml"
      ],
      "restart-commands": [
        "/usr/bin/static-pods"
      ]
    }
  }
}

Downgrade.Output

{
  "active_partition": {
    "image": {
      "arch": "x86_64",
      "variant": "aws-k8s-1.28",
      "version": "1.20.0"
    },
    "next_to_boot": true
  },
  "available_updates": [
    "1.20.0",
    "1.19.4"
  ],
  "chosen_update": {
    "arch": "x86_64",
    "variant": "aws-k8s-1.28",
    "version": "1.19.4"
  },
  "most_recent_command": {
    "cmd_status": "Success",
    "cmd_type": "refresh",
    "exit_status": 0,
    "stderr": "",
    "timestamp": "2024-04-18T23:03:03.623073844Z"
  },
  "staging_partition": null,
  "update_state": "Available"
}
23:03:02 [INFO] Refreshing updates...
23:03:03 [INFO] Downloading and applying update to disk...
23:03:08 [INFO] Still waiting for updated status, will wait up to 594.5s longer...
23:03:13 [INFO] Still waiting for updated status, will wait up to 589.5s longer...
23:03:18 [INFO] Still waiting for updated status, will wait up to 584.5s longer...
23:03:23 [INFO] Still waiting for updated status, will wait up to 579.5s longer...
23:03:26 [INFO] Setting the update active so it will apply on the next reboot...
23:03:27 [INFO] Rebooting, goodbye...

Downgrade_Build_ID.Output

{
  "os": {
    "arch": "x86_64",
    "build_id": "4f0a078e",
    "pretty_name": "Bottlerocket OS 1.19.4 (aws-k8s-1.28)",
    "variant_id": "aws-k8s-1.28",
    "version_id": "1.19.4"
  }
}

Downgrade_Config.Output

cat: /etc/kubernetes/static-pods-manifest.toml: No such file or directory

Note: Manually checked that the helper created the one.yaml file instead

configuration-files.static-pods-toml

{}

services.static-pods

{
  "services": {
    "static-pods": {
      "configuration-files": [],
      "restart-commands": [
        "/usr/bin/static-pods"
      ]
    }
  }
}

Upgrade.Output

{
  "active_partition": {
    "image": {
      "arch": "x86_64",
      "variant": "aws-k8s-1.28",
      "version": "1.19.4"
    },
    "next_to_boot": true
  },
  "available_updates": [
    "1.20.0",
    "1.19.4"
  ],
  "chosen_update": {
    "arch": "x86_64",
    "variant": "aws-k8s-1.28",
    "version": "1.20.0"
  },
  "most_recent_command": {
    "cmd_status": "Success",
    "cmd_type": "refresh",
    "exit_status": 0,
    "stderr": "",
    "timestamp": "2024-04-18T23:04:23.641833002Z"
  },
  "staging_partition": null,
  "update_state": "Available"
}
23:04:22 [INFO] Refreshing updates...
23:04:23 [INFO] Downloading and applying update to disk...
23:04:28 [INFO] Still waiting for updated status, will wait up to 594.5s longer...
23:04:33 [INFO] Still waiting for updated status, will wait up to 589.5s longer...
23:04:38 [INFO] Still waiting for updated status, will wait up to 584.5s longer...
23:04:43 [INFO] Still waiting for updated status, will wait up to 579.5s longer...
23:04:45 [INFO] Setting the update active so it will apply on the next reboot...
23:04:46 [INFO] Rebooting, goodbye...

Upgrade_Build_ID.Output

{
  "os": {
    "arch": "x86_64",
    "build_id": "cf7a4e6e",
    "pretty_name": "Bottlerocket OS 1.20.0 (aws-k8s-1.28)",
    "variant_id": "aws-k8s-1.28",
    "version_id": "1.20.0"
  }
}

Upgrade_Config.Output

/etc/kubernetes/static-pods-manifest.toml

["one"]
enabled = true
manifest = "YXBpVmVyc2lvbjogdjEKa2luZDogUG9kCm1ldGFkYXRhOgogIG5hbWU6IHN0YXRpYy13ZWIKc3BlYzoKICBjb250YWluZXJzOgogICAgLSBuYW1lOiB3ZWIKICAgICAgaW1hZ2U6IG5naW54"
["two"]
enabled = true
manifest = "YXBpVmVyc2lvbjogdjEKa2luZDogUG9kCm1ldGFkYXRhOgogIG5hbWU6IHN0YXRpYy13ZWIyCnNwZWM6CiAgY29udGFpbmVyczoKICAgIC0gbmFtZTogd2ViCiAgICAgIGltYWdlOiBuZ2lueA=="

Note: Manually tested the removal, addition, disabling of multiple static-pods

configuration-files.static-pods-toml

{
  "configuration-files": {
    "static-pods-toml": {
      "path": "/etc/kubernetes/static-pods-manifest.toml",
      "template-path": "/usr/share/templates/static-pods-toml"
    }
  }
}
{
  "services": {
    "static-pods": {
      "configuration-files": [
        "static-pods-toml"
      ],
      "restart-commands": [
        "/usr/bin/static-pods"
      ]
    }
  }
}

Change_Settings.Output

Manually ran:

apiclient set kubernetes.static-pods.two.enabled=false
["one"]
enabled = true
manifest = "YXBpVmVyc2lvbjogdjEKa2luZDogUG9kCm1ldGFkYXRhOgogIG5hbWU6IHN0YXRpYy13ZWIKc3BlYzoKICBjb250YWluZXJzOgogICAgLSBuYW1lOiB3ZWIKICAgICAgaW1hZ2U6IG5naW54"
["two"]
enabled = false
manifest = "YXBpVmVyc2lvbjogdjEKa2luZDogUG9kCm1ldGFkYXRhOgogIG5hbWU6IHN0YXRpYy13ZWIyCnNwZWM6CiAgY29udGFpbmVyczoKICAgIC0gbmFtZTogd2ViCiAgICAgIGltYWdlOiBuZ2lueA=="

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@jmt-lab jmt-lab force-pushed the ootb/static-pods branch 2 times, most recently from e231f63 to 83a755d Compare April 11, 2024 23:07
@jmt-lab jmt-lab changed the title static-pods: remove conditional compilati9on static-pods: remove conditional compilation Apr 11, 2024
@jmt-lab jmt-lab self-assigned this Apr 11, 2024
@jmt-lab jmt-lab marked this pull request as ready for review April 11, 2024 23:29
bcressey
bcressey previously approved these changes Apr 12, 2024
Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-wise this looks good.

For testing, I'd like to be sure that cluster provisioning still works, as that depends on static pods functioning as expected.

I'd also like to understand whether enabling a new static pod or disabling an existing one causes any impact to other running pods. I would expect reconciliation to happen as for anything else in Kubernetes, but static pods have an unusual lifecycle and don't always follow the same rules.

webern
webern previously requested changes Apr 12, 2024
Copy link
Member

@webern webern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to push the small fixes today if possible.

@jmt-lab jmt-lab force-pushed the ootb/static-pods branch 3 times, most recently from 300b619 to 1ae7893 Compare April 12, 2024 21:48
@jmt-lab jmt-lab requested a review from webern April 12, 2024 21:57
@jmt-lab
Copy link
Contributor Author

jmt-lab commented Apr 17, 2024

So unfortunately testing showed a few problems. First kubernetes does not actually support defining multiple pods in a single manifest file for static pods. It does not error with it but will only spin up the first defined pod in the manifest file as this is because it maps a 1 to 1 relationship from manifest file and static pod.

I then added a split-by config in thar-be-settings to have it divide a template amongst multiple files such as manifest.yaml.0 manifest.yaml.1. This works on initial creation but when disabling or removing a pod, it does not remove and clean up the file. This could be fixed by more expansion of thar-be-settings functionality, but at this point I think that is too risky of a move right now. IMO going back to keeping the agent with a toml configuration file is the saner approach, and maintaining it having that logic that already exists.

@jmt-lab
Copy link
Contributor Author

jmt-lab commented Apr 18, 2024

Fixed missing newline and serde requirement

@jmt-lab jmt-lab dismissed webern’s stale review April 18, 2024 22:53

The requested change was made, but github won't mark it done cause force push removed it from history??

packages/static-pods/Cargo.toml Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for future reviewers: easiest way to review this is to checkout the PR branch locally and

git diff 396a042e:sources/api/static-pods/src/static_pods.rs 7959b6e9:sources/api/static-pods/src/main.rs

@jmt-lab
Copy link
Contributor Author

jmt-lab commented Apr 18, 2024

Fix source-groups and migration spacing

@jmt-lab jmt-lab dismissed bcressey’s stale review April 18, 2024 23:27

Approach changed since discussion, we did talk about going this route I believe should the direct to template didn't work. More info is on PR

@jmt-lab jmt-lab requested a review from gthao313 April 18, 2024 23:28
@jmt-lab jmt-lab merged commit 41d9aa5 into bottlerocket-os:develop Apr 19, 2024
33 checks passed
@jmt-lab jmt-lab deleted the ootb/static-pods branch April 19, 2024 17:47
Comment on lines +8 to +11
[package.metadata.build-package]
source-groups = [
"api/static-pods"
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If static-pods no longer depends on the API, it should move out from under sources/api so that this package can watch the specific directory (sources/static-pods) and so that changes don't also trigger a rebuild of os.

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.

OOTB: Remove conditional compilation and model dependency from static-pods
5 participants