k8s static pods support #1317

etungsten · 2021-02-11T00:32:46Z

Issue number:
N/A

Description of changes:

Author: Erikson Tung <etung@amazon.com>
Date:   Tue Feb 9 13:20:22 2021 -0800

    settings, static-pods: new settings and helper for k8s static pods
    
    Adds a set of new settings for managing k8s static pods.
    Adds a new binary helper to manage k8s static pod manifests.

Author: Erikson Tung <etung@amazon.com>
Date:   Tue Feb 9 13:27:48 2021 -0800

    aws-k8s-*: specify 'staticPodPath' in kubelet config
    
    Specify pod manifest path in kubelet config for all kubernetes variants.

Author: Erikson Tung <etung@amazon.com>
Date:   Fri Feb 12 12:17:29 2021 -0800

    pluto: only build for kubernetes variants
    
    We only need to build pluto for kubernetes variants

Author: Erikson Tung <etung@amazon.com>
Date:   Fri Feb 12 13:23:43 2021 -0800

    Makefile: conditionally exclude packages from cargo test
    
    In tasks.unit-test, we need to conditionally exclude variant specific
    packages from being compiled if the variant does not match up

Author: Erikson Tung <etung@amazon.com>
Date:   Tue Feb 16 10:41:34 2021 -0800

    migrations: add `add-static-pods` migration
    
    Adds a new migration for new k8s static-pods settings

Testing done:

Verified that buildsys does not build the package for non-k8s variants.

Build aws-k8s-1.15, aws-k8s-1.16, aws-k8s-1.17, aws-k8s-1.18, aws-k8s-1.19 images and launched instances for all of them

Kubelet started up fine.

Then I specified a static pod that ran nginx on port 80 with host networking:

apiclient set kubernetes.static-pods.control-plane-stuff.manifest=YXBpVmVyc2lvbjogdjEKa2luZDogUG9kCm1ldGFkYXRhOgogICBuYW1lOiBzdGF0aWMtcG9kLW5naW54LXRlc3QKc3BlYzoKICAgaG9zdE5ldHdvcms6IHRydWUKICAgY29udGFpbmVyczoKICAgICAgLSBuYW1lOiBuZ2lueAogICAgICAgIGltYWdlOiBuZ2lueDoxLjE0LjIKICAgICAgICBwb3J0czoKICAgICAgICAgICAtIGNvbnRhaW5lclBvcnQ6IDgwCg==
apiclient set kubernetes.static-pods.control-plane-stuff.enabled=true

The base64 blob decodes to the following:

apiVersion: v1
kind: Pod
metadata:
   name: static-pod-nginx-test
spec:
   hostNetwork: true
   containers:
      - name: nginx
        image: nginx:1.14.2
        ports:
           - containerPort: 80

Observed that the manifest gets created successfully in the pod manifest path:

bash-5.0# cat /etc/kubernetes/static-pods/control-plane-stuff 
apiVersion: v1
kind: Pod
metadata:
   name: static-pod-nginx-test
spec:
   hostNetwork: true
   containers:
      - name: nginx
        image: nginx:1.14.2
        ports:
           - containerPort: 80

Then curling localhost:80 from within the admin container I can see nginx is running as expected:

[ec2-user@ip-192-168-3-187 ~]$ curl localhost:80
<!DOCTYPE html>
<html>
<head>
<title>Welcome to nginx!</title>
<style>
    body {
        width: 35em;
        margin: 0 auto;
        font-family: Tahoma, Verdana, Arial, sans-serif;
    }
</style>
</head>
<body>
<h1>Welcome to nginx!</h1>
<p>If you see this page, the nginx web server is successfully installed and
working. Further configuration is required.</p>

<p>For online documentation and support please refer to
<a href="http://nginx.org/">nginx.org</a>.<br/>
Commercial support is available at
<a href="http://nginx.com/">nginx.com</a>.</p>

<p><em>Thank you for using nginx.</em></p>
</body>
</html>
[ec2-user@ip-192-168-3-187 ~]$

Checking pods via kubectl, I can see mirror pod entries in the k8s API:

$ kubectl get pods -A
NAMESPACE      NAME                                                                READY   STATUS    RESTARTS   AGE
default        static-pod-nginx-test-ip-192-168-3-187.us-west-2.compute.internal   1/1     Running   0          6m16s
default        static-pod-nginx-test-ip-192-168-8-47.us-west-2.compute.internal    1/1     Running   0          20m
....

Disabling the static pod by setting enabled to false also works as expected. The manifest file gets correctly deleted.

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.

etungsten · 2021-02-11T17:54:16Z

Push above adds some documentation to the settings section of the top-level README.

README.md

packages/kubernetes-1.15/kubernetes-tmpfiles.conf

packages/os/os.spec

sources/api/static-pods/src/main.rs

sources/api/static-pods/src/static_pods.rs

packages/release/release.spec

packages/os/os.spec

sources/api/static-pods/src/main.rs

sources/api/static-pods/src/static_pods.rs

zmrow

🎼

srgothi92

Nice 🎉

Just out of curiosity and may not be relevant at all to the changes.
I was wondering what would happen if someone sends incorrect manifest file ? I think Kubernetes would not launch a pod for it. However, how can someone debug such cases.

etungsten · 2021-02-12T21:23:18Z

Nice tada

Just out of curiosity and may not be relevant at all to the changes.
I was wondering what would happen if someone sends incorrect manifest file ? I think Kubernetes would not launch a pod for it. However, how can someone debug such cases.

If a user sends a poorly formatted manifest file, kubelet would just ignore that manifest. To debug, by virtue of not seeing the pod come up, the user would need to recheck their manifest. If they would like to see kubelet's log, then they'd have to go through logdog.

etungsten · 2021-02-12T21:28:20Z

The force push from fda4739 to e3e9e1a addresses @zmrow and @bcressey 's comments.
a4b0608 above fixes the build check by modifying tasks.unit-test to conditionally exclude variant specific packages if the variant isn't aligned.

Tested and things work as expected.

etungsten · 2021-02-12T21:29:31Z

I'll remove the conditional compilation from ecs-settings-applier and do the same for it in tasks.unit-tests in a separate PR.

README.md

etungsten · 2021-02-12T23:05:06Z

Push above addresses #1317 (comment)

etungsten · 2021-02-13T23:22:14Z

Push above rebases onto develop to fix merging conflicts.

README.md

sources/api/static-pods/src/main.rs

etungsten · 2021-02-16T20:09:00Z

d409af8adac486f904bc4216bdf8328fa2a31bb9 adds the migration for migrating the new static-pods settings.
The force push above addresses @samuelkarp 's comments.

Tested the migration and it works as expected on downgrades and upgrades.

webern · 2021-02-16T19:32:42Z

variants/aws-k8s-1.15/Cargo.toml

+    "static-pods",
+    "pluto"


Interesting. Not now, but I wonder if there's a k8s.spec and k8s package for these in our future.

Or a k8s-release - but pluto is AWS-specific where static-pods is not.

webern · 2021-02-16T19:36:17Z

Makefile.toml

+# Exclude Kubernetes specific packages from being built if the variant isn't Kubernetes specific
+if ! echo "${VARIANT}" | grep -q "k8s"; then
+    EXCLUDE="--exclude static-pods pluto"
+fi
+


This feels so far away from other conditionals. :(
I think this also means that I can no longer run:

cd sources export VARIANT=aws-ecs-1 && cargo build

Which makes me sad (and might have IDE ramifications, not sure). Are we sure this is how we want to do this?

There has to be a conditional somewhere for this unit test task to work. Either we continue conditionally compiling variant specific binaries, or we do this.

Yeah, I realize that's the problem we're solving for. With ecs-settings-applier, the build.rs exports a cfg and the 'guts' are not compiled unless its ecs.

The problem with your approach is that the rust workspace is no longer usable on its own and we're moving toward a reliance on the makefile for things to work at all.

Both solutions are undesirable, so I'm not demanding a change here, but inviting discussion.

Sorry for jumping in between the conversation.

cd sources export VARIANT=aws-ecs-1 && cargo build

@webern Why won't this work ? This conditional is not inside sources dir packages right ? Not able to figure out how it affects.

I'm assuming that the reason @etungsten added this to the makefile is because the pluto and static-pods packages will fail to compile unless the model package is set to use k8s settings. If we try to compile the workspace for ECS, the model will have the wrongs structs and our build will fail.

Specifically tasks.unit-tests will fail because it just compiles everything in the workspace with the same VARIANT env var.

Regarding the cd sources; export VARIANT=aws-ecs-1 && cargo build use case. I feel like that's somewhat unusual. What reasons are there for people wanting to build the entire workspace without going through the Makefile? I would understand if people wanted to build specific workspace components for testing or w.e. But if they want to build everything in the workspace for use, shouldn't they just go through building the OS package via the Makefile?

I'm OK with your original conditional compilation for static-pods if it makes the overall workspace easier to build. I like the property that these crates can be built without bringing the entire build system to bear.

I'd prefer to get to the point where the entire sources tree can be built for each model simultaneously, and although the Makefile.toml workaround doesn't preclude that, it may invite us to continue segmenting the tree in ad-hoc ways rather than driving toward that goal.

I use the workspace without bringing the build system to bear all the time. I'm not sure how to describe the use case, but having a workspace that can't be compiled with cargo build or cargo test seems to defeat the purpose of a workspace. Right now the only "gotchya" is that the VARIANT env is needed, and the error messages document this very well (I often forget, read the error then export a VARIANT).

I think the options are:

Merge this as is with the Makefile.toml changes, and no longer can we cargo build the workspace (without adding the --exclude statements depending on VARIANT), or

Change to the cfg solution that we used with ecs-settings-applier so that we can always build all of the workspace, but the incompatible code is 'commented out'.

@bcressey are you saying that you choose #1 and that, because the workspace no longer builds, we may be more motivated to fix it?

I'll revert back to conditionally compiling static-pods since that's what people seem to prefer so far.

@bcressey are you saying that you choose #1 and that, because the workspace no longer builds, we may be more motivated to fix it?

No, I was agreeing with you and I prefer #2.

sources/api/static-pods/src/main.rs

etungsten · 2021-02-16T21:27:31Z

Push above addresses @webern 's comment #1317 (comment)

Revamps arg parsing error handling. Still works as expected:

etung in api/static-pods/debug on 🌱 static-pods [$!?] 
$ ./static-pods --log-level asdasd
Invalid log level 'asdasd'
Usage: ./static-pods
            [ --socket-path PATH ]
            [ --log-level trace|debug|info|warn|error ]

    Socket path defaults to /run/api.sock
etung in api/static-pods/debug on 🌱 static-pods [$!?] 
$ ./static-pods --socket-path
Did not give argument to --socket-path
Usage: ./static-pods
            [ --socket-path PATH ]
            [ --log-level trace|debug|info|warn|error ]

    Socket path defaults to /run/api.sock
etung in api/static-pods/debug on 🌱 static-pods [$!?] 
$ ./static-pods --log-level
Did not give argument to --log-level
Usage: ./static-pods
            [ --socket-path PATH ]
            [ --log-level trace|debug|info|warn|error ]

    Socket path defaults to /run/api.sock

arnaldo2792 · 2021-02-17T00:20:57Z

sources/api/static-pods/src/main.rs

+    ensure!(
+        code.is_success(),
+        error::APIResponse {
+            method,
+            uri,
+            code,
+            response_body,
+        }
+    );


Do you actually need this? Both apiclient::raw_request and schnauzer::get_settings (through schnauzer::get_json) have this check already

etungsten · 2021-02-17T00:34:09Z

Push above addresses @samuelkarp @arnaldo2792's comment.
Use schnauzer instead of raw api requests.
Cleans up a bunch of unused errors, oops

Things still work as expected.

etungsten · 2021-02-17T00:40:47Z

Push above sheds a few unnecessary imports.

Adds a set of new settings for managing k8s static pods. Adds a new binary helper to manage k8s static pod manifests.

Specify pod manifest path in kubelet config for all kubernetes variants.

We only need to build pluto for kubernetes variants

Adds a new migration for new k8s static-pods settings

etungsten · 2021-02-17T19:41:09Z

Push above reverts back to conditionally compiling static-pods based on the VARIANT env var. Drops changes to tasks.unit-tests.

webern

I'm happier with the cfg approach than the Makefile approach for conditional compilation, but I'll survive either way :)

etungsten force-pushed the static-pods branch from 0ad2507 to 1a4fb2c Compare February 11, 2021 00:34

etungsten changed the title ~~Static pods~~ k8s static pods support Feb 11, 2021

etungsten marked this pull request as draft February 11, 2021 00:48

This comment has been minimized.

Sign in to view

etungsten force-pushed the static-pods branch 3 times, most recently from 571d157 to c5ee2a4 Compare February 11, 2021 04:05

etungsten marked this pull request as ready for review February 11, 2021 05:28

etungsten force-pushed the static-pods branch from c5ee2a4 to fda4739 Compare February 11, 2021 17:53

etungsten requested review from tjkirch, samuelkarp, bcressey and zmrow February 11, 2021 17:54

bcressey reviewed Feb 12, 2021

View reviewed changes

bcressey mentioned this pull request Feb 12, 2021

add support for gzip compressed user data to early-boot-config #1320

Closed

zmrow reviewed Feb 12, 2021

View reviewed changes

sources/api/static-pods/src/main.rs Show resolved Hide resolved

sources/api/static-pods/src/static_pods.rs Outdated Show resolved Hide resolved

sources/api/static-pods/src/static_pods.rs Outdated Show resolved Hide resolved

sources/api/static-pods/src/static_pods.rs Show resolved Hide resolved

etungsten force-pushed the static-pods branch from fda4739 to e3e9e1a Compare February 12, 2021 20:34

zmrow reviewed Feb 12, 2021

View reviewed changes

zmrow approved these changes Feb 12, 2021

View reviewed changes

srgothi92 reviewed Feb 12, 2021

View reviewed changes

etungsten requested a review from bcressey February 12, 2021 21:26

etungsten commented Feb 12, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

etungsten force-pushed the static-pods branch from a4b0608 to ccd6d3c Compare February 12, 2021 23:04

etungsten force-pushed the static-pods branch from ccd6d3c to a247105 Compare February 13, 2021 23:21

etungsten force-pushed the static-pods branch from b95fbf2 to 8542789 Compare February 16, 2021 18:48

samuelkarp reviewed Feb 16, 2021

View reviewed changes

etungsten force-pushed the static-pods branch from 8542789 to d409af8 Compare February 16, 2021 20:06

etungsten requested review from samuelkarp and bcressey February 16, 2021 20:09

webern reviewed Feb 16, 2021

View reviewed changes

etungsten force-pushed the static-pods branch from d409af8 to 8ea2514 Compare February 16, 2021 21:26

etungsten requested a review from webern February 16, 2021 21:37

arnaldo2792 reviewed Feb 17, 2021

View reviewed changes

etungsten force-pushed the static-pods branch from 8ea2514 to e7c7518 Compare February 17, 2021 00:33

etungsten requested a review from arnaldo2792 February 17, 2021 00:34

etungsten force-pushed the static-pods branch from e7c7518 to 21484b0 Compare February 17, 2021 00:40

samuelkarp approved these changes Feb 17, 2021

View reviewed changes

etungsten added 4 commits February 17, 2021 11:27

settings, static-pods: new settings and helper for k8s static pods

fcdd9d2

Adds a set of new settings for managing k8s static pods. Adds a new binary helper to manage k8s static pod manifests.

aws-k8s-*: specify 'staticPodPath' in kubelet config

622bfa5

Specify pod manifest path in kubelet config for all kubernetes variants.

pluto: only build for kubernetes variants

d0864c3

We only need to build pluto for kubernetes variants

migrations: add add-static-pods migration

de6926b

Adds a new migration for new k8s static-pods settings

etungsten force-pushed the static-pods branch from 21484b0 to de6926b Compare February 17, 2021 19:40

webern approved these changes Feb 17, 2021

View reviewed changes

arnaldo2792 approved these changes Feb 17, 2021

View reviewed changes

bcressey approved these changes Feb 18, 2021

View reviewed changes

etungsten merged commit 9b52330 into bottlerocket-os:develop Feb 18, 2021

etungsten deleted the static-pods branch February 18, 2021 01:14

jpculp mentioned this pull request Feb 18, 2021

Add "shibaken" generator to populate user-data with public keys from IMDS #1331

Merged

rifelpet mentioned this pull request Mar 2, 2021

WIP Initial support for Bottlerocket OS kubernetes/kops#9855

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

k8s static pods support #1317

k8s static pods support #1317

etungsten commented Feb 11, 2021 •

edited

Loading

This comment has been minimized.

etungsten commented Feb 11, 2021

zmrow left a comment

srgothi92 left a comment

etungsten commented Feb 12, 2021

etungsten commented Feb 12, 2021 •

edited

Loading

etungsten commented Feb 12, 2021

etungsten commented Feb 12, 2021

etungsten commented Feb 13, 2021

etungsten commented Feb 16, 2021

webern Feb 16, 2021

bcressey Feb 17, 2021

webern Feb 16, 2021

etungsten Feb 16, 2021 •

edited

Loading

webern Feb 16, 2021 •

edited

Loading

srgothi92 Feb 16, 2021

webern Feb 16, 2021

etungsten Feb 16, 2021 •

edited

Loading

bcressey Feb 17, 2021

webern Feb 17, 2021 •

edited

Loading

etungsten Feb 17, 2021

bcressey Feb 18, 2021 •

edited

Loading

etungsten commented Feb 16, 2021 •

edited

Loading

arnaldo2792 Feb 17, 2021

etungsten commented Feb 17, 2021 •

edited

Loading

etungsten commented Feb 17, 2021

etungsten commented Feb 17, 2021

webern left a comment

k8s static pods support #1317

k8s static pods support #1317

Conversation

etungsten commented Feb 11, 2021 • edited Loading

This comment has been minimized.

etungsten commented Feb 11, 2021

zmrow left a comment

Choose a reason for hiding this comment

srgothi92 left a comment

Choose a reason for hiding this comment

etungsten commented Feb 12, 2021

etungsten commented Feb 12, 2021 • edited Loading

etungsten commented Feb 12, 2021

etungsten commented Feb 12, 2021

etungsten commented Feb 13, 2021

etungsten commented Feb 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etungsten Feb 16, 2021 • edited Loading

Choose a reason for hiding this comment

webern Feb 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etungsten Feb 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webern Feb 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcressey Feb 18, 2021 • edited Loading

Choose a reason for hiding this comment

etungsten commented Feb 16, 2021 • edited Loading

Choose a reason for hiding this comment

etungsten commented Feb 17, 2021 • edited Loading

etungsten commented Feb 17, 2021

etungsten commented Feb 17, 2021

webern left a comment

Choose a reason for hiding this comment

etungsten commented Feb 11, 2021 •

edited

Loading

etungsten commented Feb 12, 2021 •

edited

Loading

etungsten Feb 16, 2021 •

edited

Loading

webern Feb 16, 2021 •

edited

Loading

etungsten Feb 16, 2021 •

edited

Loading

webern Feb 17, 2021 •

edited

Loading

bcressey Feb 18, 2021 •

edited

Loading

etungsten commented Feb 16, 2021 •

edited

Loading

etungsten commented Feb 17, 2021 •

edited

Loading