From 202e4409203e8a8bfca63f510cd8c6c1376fc6ac Mon Sep 17 00:00:00 2001 From: Ole Markus With Date: Sun, 18 Apr 2021 16:25:28 +0200 Subject: [PATCH 1/2] Pre-pull cilium and kube-proxy in warming mode --- nodeup/pkg/model/kube_proxy.go | 22 ++- nodeup/pkg/model/kube_proxy_test.go | 8 + nodeup/pkg/model/networking/cilium.go | 12 +- .../tests/golden/minimal/tasks-warmpool.yaml | 143 ++++++++++++++++++ upup/pkg/fi/nodeup/nodetasks/BUILD.bazel | 1 + upup/pkg/fi/nodeup/nodetasks/pull_image.go | 100 ++++++++++++ upup/pkg/fi/nodeup/nodetasks/service.go | 2 +- 7 files changed, 280 insertions(+), 8 deletions(-) create mode 100644 nodeup/pkg/model/tests/golden/minimal/tasks-warmpool.yaml create mode 100644 upup/pkg/fi/nodeup/nodetasks/pull_image.go diff --git a/nodeup/pkg/model/kube_proxy.go b/nodeup/pkg/model/kube_proxy.go index aca6efd03f295..53069d9435b36 100644 --- a/nodeup/pkg/model/kube_proxy.go +++ b/nodeup/pkg/model/kube_proxy.go @@ -61,6 +61,14 @@ func (b *KubeProxyBuilder) Build(c *fi.ModelBuilderContext) error { } } + if b.ConfigurationMode == "Warming" { + pullTask := &nodetasks.PullImageTask{ + Name: kubeProxyImage(b.NodeupModelContext), + Runtime: b.Cluster.Spec.ContainerRuntime, + } + c.AddTask(pullTask) + } + { pod, err := b.buildPod() if err != nil { @@ -185,11 +193,7 @@ func (b *KubeProxyBuilder) buildPod() (*v1.Pod, error) { flags = append(flags, `--resource-container=""`) } - image := c.Image - if b.Architecture != architectures.ArchitectureAmd64 { - image = strings.Replace(image, "-amd64", "-"+string(b.Architecture), 1) - } - + image := kubeProxyImage(b.NodeupModelContext) container := &v1.Container{ Name: "kube-proxy", Image: image, @@ -312,3 +316,11 @@ func tolerateMasterTaints() []v1.Toleration { return tolerations } + +func kubeProxyImage(b *NodeupModelContext) string { + image := b.Cluster.Spec.KubeProxy.Image + if b.Architecture != architectures.ArchitectureAmd64 { + image = strings.Replace(image, "-amd64", "-"+string(b.Architecture), 1) + } + return image +} diff --git a/nodeup/pkg/model/kube_proxy_test.go b/nodeup/pkg/model/kube_proxy_test.go index 05118e5545a3d..1b235b2780a26 100644 --- a/nodeup/pkg/model/kube_proxy_test.go +++ b/nodeup/pkg/model/kube_proxy_test.go @@ -177,3 +177,11 @@ func TestKubeProxyBuilderARM64(t *testing.T) { return builder.Build(target) }) } +func TestKubeProxyBuilderWarmPool(t *testing.T) { + RunGoldenTest(t, "tests/golden/minimal", "warmpool", func(nodeupModelContext *NodeupModelContext, target *fi.ModelBuilderContext) error { + nodeupModelContext.ConfigurationMode = "Warming" + builder := KubeProxyBuilder{NodeupModelContext: nodeupModelContext} + builder.Architecture = architectures.ArchitectureArm64 + return builder.Build(target) + }) +} diff --git a/nodeup/pkg/model/networking/cilium.go b/nodeup/pkg/model/networking/cilium.go index 6289b5d4e6a23..53d4218d36ab1 100644 --- a/nodeup/pkg/model/networking/cilium.go +++ b/nodeup/pkg/model/networking/cilium.go @@ -36,7 +36,7 @@ var _ fi.ModelBuilder = &CiliumBuilder{} // Build is responsible for configuring the network cni func (b *CiliumBuilder) Build(c *fi.ModelBuilderContext) error { - networking := b.Cluster.Spec.Networking + cilium := b.Cluster.Spec.Networking.Cilium // As long as the Cilium Etcd cluster exists, we should do this if apiModel.UseCiliumEtcd(b.Cluster) { @@ -45,7 +45,7 @@ func (b *CiliumBuilder) Build(c *fi.ModelBuilderContext) error { } } - if networking.Cilium == nil { + if cilium == nil { return nil } @@ -53,6 +53,14 @@ func (b *CiliumBuilder) Build(c *fi.ModelBuilderContext) error { return err } + if b.ConfigurationMode == "Warming" { + image := &nodetasks.PullImageTask{ + Name: "docker.io/cilium/cilium:" + cilium.Version, + Runtime: b.Cluster.Spec.ContainerRuntime, + } + c.AddTask(image) + } + return nil } diff --git a/nodeup/pkg/model/tests/golden/minimal/tasks-warmpool.yaml b/nodeup/pkg/model/tests/golden/minimal/tasks-warmpool.yaml new file mode 100644 index 0000000000000..d0c2f4f266b5f --- /dev/null +++ b/nodeup/pkg/model/tests/golden/minimal/tasks-warmpool.yaml @@ -0,0 +1,143 @@ +contents: | + apiVersion: v1 + kind: Pod + metadata: + annotations: + scheduler.alpha.kubernetes.io/critical-pod: "" + creationTimestamp: null + labels: + k8s-app: kube-proxy + tier: node + name: kube-proxy + namespace: kube-system + spec: + containers: + - args: + - --cluster-cidr=100.96.0.0/11 + - --conntrack-max-per-core=131072 + - --hostname-override=@aws + - --kubeconfig=/var/lib/kube-proxy/kubeconfig + - --master=https://127.0.0.1 + - --oom-score-adj=-998 + - --v=2 + - --logtostderr=false + - --alsologtostderr + - --log-file=/var/log/kube-proxy.log + command: + - /usr/local/bin/kube-proxy + image: k8s.gcr.io/kube-proxy:v1.18.0 + name: kube-proxy + resources: + requests: + cpu: 100m + securityContext: + privileged: true + volumeMounts: + - mountPath: /var/log/kube-proxy.log + name: logfile + - mountPath: /var/lib/kube-proxy/kubeconfig + name: kubeconfig + readOnly: true + - mountPath: /lib/modules + name: modules + readOnly: true + - mountPath: /etc/ssl/certs + name: ssl-certs-hosts + readOnly: true + - mountPath: /run/xtables.lock + name: iptableslock + hostNetwork: true + priorityClassName: system-node-critical + tolerations: + - key: CriticalAddonsOnly + operator: Exists + volumes: + - hostPath: + path: /var/log/kube-proxy.log + name: logfile + - hostPath: + path: /var/lib/kube-proxy/kubeconfig + name: kubeconfig + - hostPath: + path: /lib/modules + name: modules + - hostPath: + path: /usr/share/ca-certificates + name: ssl-certs-hosts + - hostPath: + path: /run/xtables.lock + type: FileOrCreate + name: iptableslock + status: {} +path: /etc/kubernetes/manifests/kube-proxy.manifest +type: file +--- +beforeServices: +- kubelet.service +contents: + task: + CA: + task: + Name: kube-proxy + signer: ca + subject: + CommonName: system:kube-proxy + type: client + Cert: + task: + Name: kube-proxy + signer: ca + subject: + CommonName: system:kube-proxy + type: client + Key: + task: + Name: kube-proxy + signer: ca + subject: + CommonName: system:kube-proxy + type: client + Name: kube-proxy + ServerURL: https://127.0.0.1 +mode: "0400" +path: /var/lib/kube-proxy/kubeconfig +type: file +--- +contents: "" +ifNotExists: true +mode: "0400" +path: /var/log/kube-proxy.log +type: file +--- +Name: kube-proxy +signer: ca +subject: + CommonName: system:kube-proxy +type: client +--- +CA: + task: + Name: kube-proxy + signer: ca + subject: + CommonName: system:kube-proxy + type: client +Cert: + task: + Name: kube-proxy + signer: ca + subject: + CommonName: system:kube-proxy + type: client +Key: + task: + Name: kube-proxy + signer: ca + subject: + CommonName: system:kube-proxy + type: client +Name: kube-proxy +ServerURL: https://127.0.0.1 +--- +Name: k8s.gcr.io/kube-proxy:v1.18.0 +Runtime: docker diff --git a/upup/pkg/fi/nodeup/nodetasks/BUILD.bazel b/upup/pkg/fi/nodeup/nodetasks/BUILD.bazel index f21435d26870b..1daa662d71205 100644 --- a/upup/pkg/fi/nodeup/nodetasks/BUILD.bazel +++ b/upup/pkg/fi/nodeup/nodetasks/BUILD.bazel @@ -14,6 +14,7 @@ go_library( "kubeconfig.go", "load_image.go", "package.go", + "pull_image.go", "service.go", "update_packages.go", "user.go", diff --git a/upup/pkg/fi/nodeup/nodetasks/pull_image.go b/upup/pkg/fi/nodeup/nodetasks/pull_image.go new file mode 100644 index 0000000000000..a9f5569380020 --- /dev/null +++ b/upup/pkg/fi/nodeup/nodetasks/pull_image.go @@ -0,0 +1,100 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package nodetasks + +import ( + "fmt" + "os/exec" + "strings" + + "k8s.io/klog/v2" + "k8s.io/kops/upup/pkg/fi" + "k8s.io/kops/upup/pkg/fi/nodeup/local" +) + +// PullImageTask is responsible for pulling a docker image +type PullImageTask struct { + Name string + Runtime string +} + +var _ fi.Task = &PullImageTask{} +var _ fi.HasDependencies = &PullImageTask{} + +func (t *PullImageTask) GetDependencies(tasks map[string]fi.Task) []fi.Task { + // LoadImageTask depends on the docker service to ensure we + // sideload images after docker is completely updated and + // configured. + var deps []fi.Task + for _, v := range tasks { + if svc, ok := v.(*Service); ok && svc.Name == containerdService { + deps = append(deps, v) + } + if svc, ok := v.(*Service); ok && svc.Name == dockerService { + deps = append(deps, v) + } + } + return deps +} + +func (e *PullImageTask) Find(c *fi.Context) (*PullImageTask, error) { + klog.Warningf("LoadImageTask checking if image present not yet implemented") + return nil, nil +} + +func (e *PullImageTask) Run(c *fi.Context) error { + return fi.DefaultDeltaRunMethod(e, c) +} + +func (t *PullImageTask) GetName() *string { + if t.Name == "" { + return nil + } + return &t.Name +} + +func (*PullImageTask) CheckChanges(a, e, changes *PullImageTask) error { + return nil +} + +func (*PullImageTask) RenderLocal(t *local.LocalTarget, a, e, changes *PullImageTask) error { + runtime := e.Runtime + if runtime != "docker" && runtime != "containerd" { + return fmt.Errorf("no runtime specified") + } + + // Load the container image + var args []string + switch runtime { + case "docker": + args = []string{"docker", "pull", e.Name} + case "containerd": + args = []string{"ctr", "--namespace", "k8s.io", "images", "pull", e.Name} + default: + return fmt.Errorf("unknown container runtime: %s", runtime) + } + human := strings.Join(args, " ") + + klog.Infof("running command %s", human) + cmd := exec.Command(args[0], args[1:]...) + output, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("error pulling docker image with '%s': %v: %s", human, err, string(output)) + } + + return nil +} diff --git a/upup/pkg/fi/nodeup/nodetasks/service.go b/upup/pkg/fi/nodeup/nodetasks/service.go index 32337780c193f..f02770bb4e3ee 100644 --- a/upup/pkg/fi/nodeup/nodetasks/service.go +++ b/upup/pkg/fi/nodeup/nodetasks/service.go @@ -75,7 +75,7 @@ func (p *Service) GetDependencies(tasks map[string]fi.Task) []fi.Task { switch v := v.(type) { case *Package, *UpdatePackages, *UserTask, *GroupTask, *Chattr, *BindMount, *Archive: deps = append(deps, v) - case *Service, *LoadImageTask, *IssueCert, *BootstrapClientTask, *KubeConfig: + case *Service, *LoadImageTask, *PullImageTask, *IssueCert, *BootstrapClientTask, *KubeConfig: // ignore case *File: if len(v.BeforeServices) > 0 { From df4f429ceb6bc3dbdc0e06397778896dde5e4159 Mon Sep 17 00:00:00 2001 From: Ole Markus With Date: Sun, 18 Apr 2021 19:39:37 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: John Gardiner Myers --- nodeup/pkg/model/context.go | 10 ++++++++++ nodeup/pkg/model/kube_proxy.go | 8 +------- nodeup/pkg/model/networking/cilium.go | 10 +++------- upup/pkg/fi/nodeup/nodetasks/pull_image.go | 22 ++++------------------ 4 files changed, 18 insertions(+), 32 deletions(-) diff --git a/nodeup/pkg/model/context.go b/nodeup/pkg/model/context.go index 008b6f9c06964..5c43397d94e99 100644 --- a/nodeup/pkg/model/context.go +++ b/nodeup/pkg/model/context.go @@ -644,3 +644,13 @@ func (c *NodeupModelContext) CNIBinDir() string { func (c *NodeupModelContext) CNIConfDir() string { return "/etc/cni/net.d/" } + +func (c *NodeupModelContext) WarmPullImage(ctx *fi.ModelBuilderContext, imageName string) { + if c.ConfigurationMode == "Warming" { + image := &nodetasks.PullImageTask{ + Name: imageName, + Runtime: c.Cluster.Spec.ContainerRuntime, + } + ctx.AddTask(image) + } +} diff --git a/nodeup/pkg/model/kube_proxy.go b/nodeup/pkg/model/kube_proxy.go index 53069d9435b36..b6f69f15f8e68 100644 --- a/nodeup/pkg/model/kube_proxy.go +++ b/nodeup/pkg/model/kube_proxy.go @@ -61,13 +61,7 @@ func (b *KubeProxyBuilder) Build(c *fi.ModelBuilderContext) error { } } - if b.ConfigurationMode == "Warming" { - pullTask := &nodetasks.PullImageTask{ - Name: kubeProxyImage(b.NodeupModelContext), - Runtime: b.Cluster.Spec.ContainerRuntime, - } - c.AddTask(pullTask) - } + b.WarmPullImage(c, kubeProxyImage(b.NodeupModelContext)) { pod, err := b.buildPod() diff --git a/nodeup/pkg/model/networking/cilium.go b/nodeup/pkg/model/networking/cilium.go index 53d4218d36ab1..667b694203bb4 100644 --- a/nodeup/pkg/model/networking/cilium.go +++ b/nodeup/pkg/model/networking/cilium.go @@ -53,13 +53,9 @@ func (b *CiliumBuilder) Build(c *fi.ModelBuilderContext) error { return err } - if b.ConfigurationMode == "Warming" { - image := &nodetasks.PullImageTask{ - Name: "docker.io/cilium/cilium:" + cilium.Version, - Runtime: b.Cluster.Spec.ContainerRuntime, - } - c.AddTask(image) - } + image := "docker.io/cilium/cilium:" + cilium.Version + + b.WarmPullImage(c, image) return nil diff --git a/upup/pkg/fi/nodeup/nodetasks/pull_image.go b/upup/pkg/fi/nodeup/nodetasks/pull_image.go index a9f5569380020..8b279a84cb599 100644 --- a/upup/pkg/fi/nodeup/nodetasks/pull_image.go +++ b/upup/pkg/fi/nodeup/nodetasks/pull_image.go @@ -23,7 +23,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kops/upup/pkg/fi" - "k8s.io/kops/upup/pkg/fi/nodeup/local" ) // PullImageTask is responsible for pulling a docker image @@ -36,8 +35,8 @@ var _ fi.Task = &PullImageTask{} var _ fi.HasDependencies = &PullImageTask{} func (t *PullImageTask) GetDependencies(tasks map[string]fi.Task) []fi.Task { - // LoadImageTask depends on the docker service to ensure we - // sideload images after docker is completely updated and + // ImagePullTask depends on the container runtime service to ensure we + // sideload images after the container runtime is completely updated and // configured. var deps []fi.Task for _, v := range tasks { @@ -51,15 +50,6 @@ func (t *PullImageTask) GetDependencies(tasks map[string]fi.Task) []fi.Task { return deps } -func (e *PullImageTask) Find(c *fi.Context) (*PullImageTask, error) { - klog.Warningf("LoadImageTask checking if image present not yet implemented") - return nil, nil -} - -func (e *PullImageTask) Run(c *fi.Context) error { - return fi.DefaultDeltaRunMethod(e, c) -} - func (t *PullImageTask) GetName() *string { if t.Name == "" { return nil @@ -67,17 +57,13 @@ func (t *PullImageTask) GetName() *string { return &t.Name } -func (*PullImageTask) CheckChanges(a, e, changes *PullImageTask) error { - return nil -} - -func (*PullImageTask) RenderLocal(t *local.LocalTarget, a, e, changes *PullImageTask) error { +func (e *PullImageTask) Run(c *fi.Context) error { runtime := e.Runtime if runtime != "docker" && runtime != "containerd" { return fmt.Errorf("no runtime specified") } - // Load the container image + // Pull the container image var args []string switch runtime { case "docker":