From 318df864b6065f4b9041e5bd3e52c0ac0c2ce9ed Mon Sep 17 00:00:00 2001 From: Yossi Boaron Date: Sun, 22 Mar 2020 15:36:35 +0200 Subject: [PATCH 01/20] Add support for reading API LB backends from KUBE-API This PR updates HAProxy static pod manifest to enable reading API LB backend details from Kubernetes instead of _etcd-server-ssl._tcp SRV. --- .../00-master/baremetal/files/baremetal-haproxy.yaml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/templates/master/00-master/baremetal/files/baremetal-haproxy.yaml b/templates/master/00-master/baremetal/files/baremetal-haproxy.yaml index e48ce1d94b..78cc8b9283 100644 --- a/templates/master/00-master/baremetal/files/baremetal-haproxy.yaml +++ b/templates/master/00-master/baremetal/files/baremetal-haproxy.yaml @@ -17,9 +17,9 @@ contents: - name: resource-dir hostPath: path: "/etc/kubernetes/static-pod-resources/haproxy" - - name: kubeconfig + - name: kubeconfigvarlib hostPath: - path: "/etc/kubernetes/kubeconfig" + path: "/var/lib/kubelet" - name: run-dir empty-dir: {} - name: conf-dir @@ -111,7 +111,7 @@ contents: image: {{ .Images.baremetalRuntimeCfgImage }} command: - monitor - - "/etc/kubernetes/kubeconfig" + - "/var/lib/kubelet/kubeconfig" - "/config/haproxy.cfg.tmpl" - "/etc/haproxy/haproxy.cfg" - "--api-vip" @@ -129,8 +129,8 @@ contents: mountPath: "/config" - name: chroot-host mountPath: "/host" - - name: kubeconfig - mountPath: "/etc/kubernetes/kubeconfig" + - name: kubeconfigvarlib + mountPath: "/var/lib/kubelet" terminationMessagePolicy: FallbackToLogsOnError imagePullPolicy: IfNotPresent hostNetwork: true From ba80b7dd0843558f76073484625fd960e36557b2 Mon Sep 17 00:00:00 2001 From: Joel Smith Date: Wed, 25 Mar 2020 11:57:04 -0600 Subject: [PATCH 02/20] Move non-existant runtimeCgroups kubelet option to flag --- templates/master/01-master-kubelet/_base/files/kubelet.yaml | 1 - templates/master/01-master-kubelet/_base/units/kubelet.yaml | 1 + templates/worker/01-worker-kubelet/_base/files/kubelet.yaml | 1 - templates/worker/01-worker-kubelet/_base/units/kubelet.yaml | 1 + 4 files changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/master/01-master-kubelet/_base/files/kubelet.yaml b/templates/master/01-master-kubelet/_base/files/kubelet.yaml index ea6f324cc2..96ac89734d 100644 --- a/templates/master/01-master-kubelet/_base/files/kubelet.yaml +++ b/templates/master/01-master-kubelet/_base/files/kubelet.yaml @@ -20,7 +20,6 @@ contents: kubeAPIQPS: 50 kubeAPIBurst: 100 rotateCertificates: true - runtimeCgroups: /system.slice/crio.service serializeImagePulls: false staticPodPath: /etc/kubernetes/manifests systemCgroups: /system.slice diff --git a/templates/master/01-master-kubelet/_base/units/kubelet.yaml b/templates/master/01-master-kubelet/_base/units/kubelet.yaml index b9af41c9f2..83d602e443 100644 --- a/templates/master/01-master-kubelet/_base/units/kubelet.yaml +++ b/templates/master/01-master-kubelet/_base/units/kubelet.yaml @@ -22,6 +22,7 @@ contents: | --kubeconfig=/var/lib/kubelet/kubeconfig \ --container-runtime=remote \ --container-runtime-endpoint=/var/run/crio/crio.sock \ + --runtime-cgroups=/system.slice/crio.service \ --node-labels=node-role.kubernetes.io/master,node.openshift.io/os_id=${ID} \ {{- if .KubeletIPv6}} --node-ip :: \ diff --git a/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml b/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml index ea6f324cc2..96ac89734d 100644 --- a/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml +++ b/templates/worker/01-worker-kubelet/_base/files/kubelet.yaml @@ -20,7 +20,6 @@ contents: kubeAPIQPS: 50 kubeAPIBurst: 100 rotateCertificates: true - runtimeCgroups: /system.slice/crio.service serializeImagePulls: false staticPodPath: /etc/kubernetes/manifests systemCgroups: /system.slice diff --git a/templates/worker/01-worker-kubelet/_base/units/kubelet.yaml b/templates/worker/01-worker-kubelet/_base/units/kubelet.yaml index 990ed9bc31..4930ceb832 100644 --- a/templates/worker/01-worker-kubelet/_base/units/kubelet.yaml +++ b/templates/worker/01-worker-kubelet/_base/units/kubelet.yaml @@ -22,6 +22,7 @@ contents: | --kubeconfig=/var/lib/kubelet/kubeconfig \ --container-runtime=remote \ --container-runtime-endpoint=/var/run/crio/crio.sock \ + --runtime-cgroups=/system.slice/crio.service \ --node-labels=node-role.kubernetes.io/worker,node.openshift.io/os_id=${ID} \ {{- if .KubeletIPv6}} --node-ip :: \ From dfafaafc903b5cd62c36d620cf81fd6ffe66b1d1 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 26 Mar 2020 11:38:41 +0100 Subject: [PATCH 03/20] pkg/daemon: fix deletion of stale files We have a serious bug in how we backup "original" files and restore them. Here, "original" means files that ship with RHCOS. Think of a default Chrony or another system daemon configuration file. When the MCD kicks in and writes to those files, we want to backup the original one (the shipped-with-RHCOS) in order to restore it if a user deletes the MC that modified it (this was the initial bug reported in GitHub at https://github.com/openshift/machine-config-operator/issues/782). However, that patch that fixed https://github.com/openshift/machine-config-operator/issues/782 was causing the following; if you shipped a file with just _one_ MC, removing it would wipe it out and that works. However, if you modified that file later again with another MC, a backup file will be created for the first MC, and when deleting the file by deleting the second MC, it will restore the initial file shipped with the first MC instead of wiping it out completely which it should have since that file was never meant to be backed up because it wasn't on RHCOS from the beginning. This patch now differentiates between files that are already on RHCOS (on-disk so to speak) and files that are shipped with an MC. For the former, the MCD will create a backup as it's doing today, for the latter instead, the MCD creates a placeholder file that tells it to just get rid of the file altogether (along with adding all the necessary checks and actions in order to create those backup files). The issue popped up on upgrade paths where the new manifests rendered by the MCO don't contain a certain file. The MCD notices that and go ahead trying to remove the file. It however notices that a backup file (which was created for an MC shipped file and later other MC have modified it) is there and tries to restore it (also failing with invalid cross-link device error, but that's another issue which I'm fixing here as well by using cp directly). Really hoping all the above makes sense. Signed-off-by: Antonio Murdaca --- pkg/daemon/update.go | 55 +++++++++++++------------------------------- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index cf4f68a058..b3437c46fe 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -806,33 +806,17 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *ignTypes.Config) e for _, f := range oldIgnConfig.Storage.Files { if _, ok := newFileSet[f.Path]; !ok { - if _, err := os.Stat(noOrigFileStampName(f.Path)); err == nil { - if err := os.Remove(noOrigFileStampName(f.Path)); err != nil { - return errors.Wrapf(err, "deleting noorig file stamp %q: %v", noOrigFileStampName(f.Path), err) + if _, err := os.Stat(noOrigFileName(f.Path)); err == nil { + if err := os.Remove(noOrigFileName(f.Path)); err != nil { + return errors.Wrapf(err, "deleting no orig file %q: %v", noOrigFileName(f.Path), err) } glog.V(2).Infof("Removing file %q completely", f.Path) } else if _, err := os.Stat(origFileName(f.Path)); err == nil { - // Add a check for backwards compatibility: basically if the file doesn't exist in /usr/etc (on FCOS/RHCOS) - // and no rpm is claiming it, we assume that the orig file came from a wrongful backup of a MachineConfig - // file instead of a file originally on disk. See https://bugzilla.redhat.com/show_bug.cgi?id=1814397 - if _, err := exec.Command("rpm", "-qf", f.Path).CombinedOutput(); err != nil { - if err := os.Remove(origFileName(f.Path)); err != nil { - return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) - } - } else if _, err := os.Stat("/usr" + f.Path); strings.HasPrefix(f.Path, "/etc") && os.IsNotExist(err) && - (operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) { - if err := os.Remove(origFileName(f.Path)); err != nil { - return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) - } - } else { - if out, err := exec.Command("cp", "-a", "--reflink=auto", origFileName(f.Path), f.Path).CombinedOutput(); err != nil { - return errors.Wrapf(err, "restoring %q from orig file %q: %s", f.Path, origFileName(f.Path), string(out)) - } - if err := os.Remove(origFileName(f.Path)); err != nil { - return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) - } - glog.V(2).Infof("Restored file %q", f.Path) - continue + if out, err := exec.Command("cp", "-a", "--reflink=auto", origFileName(f.Path), f.Path).CombinedOutput(); err != nil { + return errors.Wrapf(err, "restoring %q from orig file %q: %s", f.Path, origFileName(f.Path), string(out)) + } + if err := os.Remove(origFileName(f.Path)); err != nil { + return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) } } glog.V(2).Infof("Deleting stale config file: %s", f.Path) @@ -1038,30 +1022,23 @@ func origFileName(fpath string) string { return filepath.Join(origParentDir(), fpath+".mcdorig") } -// We use this to create a file that indicates that no original file existed on disk -// when we write a file via a MachineConfig. Otherwise the MCD does not differentiate -// between "a file existed due to a previous machineconfig" vs "a file existed on disk -// before the MCD took over". Also see deleteStaleData() above. -// -// The "stamp" part of the name indicates it is not an actual backup file, just an -// empty file to indicate lack of previous existence. -func noOrigFileStampName(fpath string) string { +func noOrigFileName(fpath string) string { return filepath.Join(noOrigParentDir(), fpath+".mcdnoorig") } func createOrigFile(fpath string) error { - if _, err := os.Stat(noOrigFileStampName(fpath)); err == nil { + if _, err := os.Stat(noOrigFileName(fpath)); err == nil { // we already created the no orig file for this default file return nil } - if _, err := os.Stat(fpath); os.IsNotExist(err) { - // create a noorig file that tells the MCD that the file wasn't present on disk before MCD - // took over so it can just remove it when deleting stale data, as opposed as restoring a file - // that was shipped _with_ the underlying OS (e.g. a default chrony config). - if err := os.MkdirAll(filepath.Dir(noOrigFileStampName(fpath)), 0755); err != nil { + if _, err := os.Stat(fpath); err != nil { + // create a noorig file that tells the MCD that the file wasn't present on disk in RHCOS + // so it can just remove it when deleting stale data, as opposed as restoring a file + // that was shipped _with_ RHCOS (e.g. a default chrony config). + if err := os.MkdirAll(filepath.Dir(noOrigFileName(fpath)), 0755); err != nil { return errors.Wrapf(err, "creating no orig parent dir: %v", err) } - return writeFileAtomicallyWithDefaults(noOrigFileStampName(fpath), nil) + return writeFileAtomicallyWithDefaults(noOrigFileName(fpath), nil) } if _, err := os.Stat(origFileName(fpath)); err == nil { // the orig file is already there and we avoid creating a new one to preserve the real default From 6b0d56fbafdedeba7909eedb45b61e1451ebc947 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 26 Mar 2020 14:39:59 +0100 Subject: [PATCH 04/20] Makefile: use ./vendor for tests Avoids querying the network and download vendors in the CI mainly. Signed-off-by: Antonio Murdaca --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f04104f2ed..201369ca66 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,7 @@ test: test-unit test-e2e # Unit tests only (no active cluster required) test-unit: - CGO_ENABLED=0 go test -tags=$(GOTAGS) -count=1 -v ./cmd/... ./pkg/... ./lib/... + CGO_ENABLED=0 go test -mod=vendor -tags=$(GOTAGS) -count=1 -v ./cmd/... ./pkg/... ./lib/... # Run the code generation tasks. # Example: @@ -107,4 +107,4 @@ Dockerfile.rhel7: Dockerfile Makefile # This was copied from https://github.com/openshift/cluster-image-registry-operato test-e2e: - go test -timeout 120m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/ + go test -mod=vendor -timeout 120m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/ From d7a83b7c56207bf8c1cce9e02058c37184f8cb34 Mon Sep 17 00:00:00 2001 From: Joseph Callen Date: Tue, 24 Mar 2020 17:53:48 -0400 Subject: [PATCH 05/20] vsphere ipi: set hostname using vmtoolsd and VM extra config This commit adds two new files: - A systemd unit that runs a shell script - A shell script that runs vmtoolsd to retrieve `guestinfo.hostname` from vm guest. Then uses hostnamectl to set hostname. --- .../common/vsphere/files/vsphere-hostname.yaml | 14 ++++++++++++++ .../common/vsphere/units/vsphere-hostname.yaml | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 templates/common/vsphere/files/vsphere-hostname.yaml create mode 100644 templates/common/vsphere/units/vsphere-hostname.yaml diff --git a/templates/common/vsphere/files/vsphere-hostname.yaml b/templates/common/vsphere/files/vsphere-hostname.yaml new file mode 100644 index 0000000000..2107d7b704 --- /dev/null +++ b/templates/common/vsphere/files/vsphere-hostname.yaml @@ -0,0 +1,14 @@ +filesystem: "root" +mode: 0755 +path: "/usr/local/bin/vsphere-hostname.sh" +contents: + inline: | + #!/usr/bin/env bash + set -e + + if [ $(hostname -s) = "localhost" ]; then + if hostname=$(/bin/vmtoolsd --cmd 'info-get guestinfo.hostname'); then + /usr/bin/hostnamectl --transient --static set-hostname ${hostname} + fi + fi + diff --git a/templates/common/vsphere/units/vsphere-hostname.yaml b/templates/common/vsphere/units/vsphere-hostname.yaml new file mode 100644 index 0000000000..4c57b2dd6f --- /dev/null +++ b/templates/common/vsphere/units/vsphere-hostname.yaml @@ -0,0 +1,16 @@ +name: "vsphere-hostname.service" +enabled: true +contents: | + [Unit] + Description=vSphere hostname + After=vmtoolsd.service + Before=kubelet.service + + [Service] + ExecStart=/usr/local/bin/vsphere-hostname.sh + Restart=on-failure + RestartSec=15 + + [Install] + WantedBy=multi-user.target + From 1f54e64c9dc804abf86319b8d8f21887b5ab1a84 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Tue, 31 Mar 2020 11:01:24 +0200 Subject: [PATCH 06/20] pkg/operator: clear Degraded on task's success The MCO runs a bunch of sync functions during its sync. Each of these functions have a name (a task name). When we loop through the sync functions and fail on any of them, the MCO breaks and reports the Degraded status for that task as Reason: TaskNameFailed. It then goes and retry all the sync functions (no change in behavior till here). During some debugging and testing I've noticed that the RenderConfig task failed with the version mismatch error for osImageURL. That is good, and also right. The MCO went ahead and set the Degraded condition to RenderConfigFailed (correctly!). The MCO then went again and re-tried all the sync functions. Now the RenderConfig task succeeded (the version mismatch error can indeed be temporary). What happened from now on is that the MCO kept rolling and eventually finishes the upgrade I was running but the RenderConfigFailed Degraded status was never cleared back to False even if the task _did_ succeed. This patch introduces a new function which we call after every sync function in order to clear any previous degraded status when it instaed succeeded on later syncs. Signed-off-by: Antonio Murdaca --- pkg/operator/status.go | 21 +++++++++++++++++++++ pkg/operator/sync.go | 3 +++ 2 files changed, 24 insertions(+) diff --git a/pkg/operator/status.go b/pkg/operator/status.go index 6e70435e9e..9a0dacead6 100644 --- a/pkg/operator/status.go +++ b/pkg/operator/status.go @@ -163,6 +163,27 @@ const ( asExpectedReason = "AsExpected" ) +func (optr *Operator) clearDegradedStatus(task string) error { + co, err := optr.fetchClusterOperator() + if err != nil { + return err + } + if co == nil { + return nil + } + if cov1helpers.IsStatusConditionFalse(co.Status.Conditions, configv1.OperatorDegraded) { + return nil + } + degradedStatusCondition := cov1helpers.FindStatusCondition(co.Status.Conditions, configv1.OperatorDegraded) + if degradedStatusCondition == nil { + return nil + } + if degradedStatusCondition.Reason != task+"Failed" { + return nil + } + return optr.syncDegradedStatus(syncError{}) +} + // syncDegradedStatus applies the new condition to the mco's ClusterOperator object. func (optr *Operator) syncDegradedStatus(ierr syncError) (err error) { co, err := optr.fetchClusterOperator() diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 918d415a3e..82b5fd4bad 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -63,6 +63,9 @@ func (optr *Operator) syncAll(syncFuncs []syncFunc) error { if syncErr.err != nil { break } + if err := optr.clearDegradedStatus(sf.name); err != nil { + return fmt.Errorf("error clearing degraded status: %v", err) + } } if err := optr.syncDegradedStatus(syncErr); err != nil { From fb9f5fcd0deec1970e8870cba4f3aaf0d9491c88 Mon Sep 17 00:00:00 2001 From: Yu Qi Zhang Date: Thu, 26 Mar 2020 12:21:06 -0400 Subject: [PATCH 07/20] update.go: add extra checks when restoring .orig files Add a check for backwards compatibility: basically if the file doesn't exist in /usr/etc/ and no rpm is claming it, we assume that the orig file came from a wrongful backup of a MachineConfig file instead of a RHCOS file. In this case we delete the `orig` file and don't back up. Also change the name to noOrigFileStamp for clarity. Signed-off-by: Yu Qi Zhang --- pkg/daemon/update.go | 54 +++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index b3437c46fe..e710c1f1e7 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -806,17 +806,32 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *ignTypes.Config) e for _, f := range oldIgnConfig.Storage.Files { if _, ok := newFileSet[f.Path]; !ok { - if _, err := os.Stat(noOrigFileName(f.Path)); err == nil { - if err := os.Remove(noOrigFileName(f.Path)); err != nil { - return errors.Wrapf(err, "deleting no orig file %q: %v", noOrigFileName(f.Path), err) + if _, err := os.Stat(noOrigFileStampName(f.Path)); err == nil { + if err := os.Remove(noOrigFileStampName(f.Path)); err != nil { + return errors.Wrapf(err, "deleting noorig file stamp %q: %v", noOrigFileStampName(f.Path), err) } glog.V(2).Infof("Removing file %q completely", f.Path) } else if _, err := os.Stat(origFileName(f.Path)); err == nil { - if out, err := exec.Command("cp", "-a", "--reflink=auto", origFileName(f.Path), f.Path).CombinedOutput(); err != nil { - return errors.Wrapf(err, "restoring %q from orig file %q: %s", f.Path, origFileName(f.Path), string(out)) - } - if err := os.Remove(origFileName(f.Path)); err != nil { - return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) + // Add a check for backwards compatibility: basically if the file doesn't exist in /usr/etc + // and no rpm is claiming it, we assume that the orig file came from a wrongful backup of a + // MachineConfig file instead of a file originally on disk. See https://bugzilla.redhat.com/show_bug.cgi?id=1814397 + if _, err := exec.Command("rpm", "-qf", f.Path).CombinedOutput(); err != nil { + if err := os.Remove(origFileName(f.Path)); err != nil { + return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) + } + } else if _, err := os.Stat("/usr" + f.Path); strings.HasPrefix(f.Path, "/etc") && os.IsNotExist(err) { + if err := os.Remove(origFileName(f.Path)); err != nil { + return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) + } + } else { + if out, err := exec.Command("cp", "-a", "--reflink=auto", origFileName(f.Path), f.Path).CombinedOutput(); err != nil { + return errors.Wrapf(err, "restoring %q from orig file %q: %s", f.Path, origFileName(f.Path), string(out)) + } + if err := os.Remove(origFileName(f.Path)); err != nil { + return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) + } + glog.V(2).Infof("Restored file %q", f.Path) + continue } } glog.V(2).Infof("Deleting stale config file: %s", f.Path) @@ -1022,23 +1037,30 @@ func origFileName(fpath string) string { return filepath.Join(origParentDir(), fpath+".mcdorig") } -func noOrigFileName(fpath string) string { +// We use this to create a file that indicates that no original file existed on disk +// when we write a file via a MachineConfig. Otherwise the MCD does not differentiate +// between "a file existed due to a previous machineconfig" vs "a file existed on disk +// before the MCD took over". Also see deleteStaleData() above. +// +// The "stamp" part of the name indicates it is not an actual backup file, just an +// empty file to indicate lack of previous existence. +func noOrigFileStampName(fpath string) string { return filepath.Join(noOrigParentDir(), fpath+".mcdnoorig") } func createOrigFile(fpath string) error { - if _, err := os.Stat(noOrigFileName(fpath)); err == nil { + if _, err := os.Stat(noOrigFileStampName(fpath)); err == nil { // we already created the no orig file for this default file return nil } - if _, err := os.Stat(fpath); err != nil { - // create a noorig file that tells the MCD that the file wasn't present on disk in RHCOS - // so it can just remove it when deleting stale data, as opposed as restoring a file - // that was shipped _with_ RHCOS (e.g. a default chrony config). - if err := os.MkdirAll(filepath.Dir(noOrigFileName(fpath)), 0755); err != nil { + if _, err := os.Stat(fpath); os.IsNotExist(err) { + // create a noorig file that tells the MCD that the file wasn't present on disk before MCD + // took over so it can just remove it when deleting stale data, as opposed as restoring a file + // that was shipped _with_ the underlying OS (e.g. a default chrony config). + if err := os.MkdirAll(filepath.Dir(noOrigFileStampName(fpath)), 0755); err != nil { return errors.Wrapf(err, "creating no orig parent dir: %v", err) } - return writeFileAtomicallyWithDefaults(noOrigFileName(fpath), nil) + return writeFileAtomicallyWithDefaults(noOrigFileStampName(fpath), nil) } if _, err := os.Stat(origFileName(fpath)); err == nil { // the orig file is already there and we avoid creating a new one to preserve the real default From becd4729f2033af27c06b2c5df1f64412ab7fcc5 Mon Sep 17 00:00:00 2001 From: Yu Qi Zhang Date: Tue, 31 Mar 2020 11:33:47 -0400 Subject: [PATCH 08/20] deleteStaleFiles: only check for /usr/etc on *COS systems --- pkg/daemon/update.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index e710c1f1e7..cf4f68a058 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -812,14 +812,15 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *ignTypes.Config) e } glog.V(2).Infof("Removing file %q completely", f.Path) } else if _, err := os.Stat(origFileName(f.Path)); err == nil { - // Add a check for backwards compatibility: basically if the file doesn't exist in /usr/etc - // and no rpm is claiming it, we assume that the orig file came from a wrongful backup of a - // MachineConfig file instead of a file originally on disk. See https://bugzilla.redhat.com/show_bug.cgi?id=1814397 + // Add a check for backwards compatibility: basically if the file doesn't exist in /usr/etc (on FCOS/RHCOS) + // and no rpm is claiming it, we assume that the orig file came from a wrongful backup of a MachineConfig + // file instead of a file originally on disk. See https://bugzilla.redhat.com/show_bug.cgi?id=1814397 if _, err := exec.Command("rpm", "-qf", f.Path).CombinedOutput(); err != nil { if err := os.Remove(origFileName(f.Path)); err != nil { return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) } - } else if _, err := os.Stat("/usr" + f.Path); strings.HasPrefix(f.Path, "/etc") && os.IsNotExist(err) { + } else if _, err := os.Stat("/usr" + f.Path); strings.HasPrefix(f.Path, "/etc") && os.IsNotExist(err) && + (operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) { if err := os.Remove(origFileName(f.Path)); err != nil { return errors.Wrapf(err, "deleting orig file %q: %v", origFileName(f.Path), err) } From 9abae6d98d3c439728b154cd362a0a41bad25a18 Mon Sep 17 00:00:00 2001 From: Kirsten G Date: Wed, 1 Apr 2020 00:35:47 -0700 Subject: [PATCH 09/20] pkg/daemon: Add event for drain failures --- pkg/daemon/update.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index cf4f68a058..a6e2b6ed66 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -159,9 +159,11 @@ func (dn *Daemon) drain() error { if err == wait.ErrWaitTimeout { failMsg := fmt.Sprintf("%d tries: %v", backoff.Steps, lastErr) MCDDrainErr.WithLabelValues(failTime, failMsg).SetToCurrentTime() + dn.recorder.Eventf(getNodeRef(dn.node), corev1.EventTypeWarning, "FailedToDrain", failMsg) return errors.Wrapf(lastErr, "failed to drain node (%d tries): %v", backoff.Steps, err) } MCDDrainErr.WithLabelValues(failTime, err.Error()).SetToCurrentTime() + dn.recorder.Eventf(getNodeRef(dn.node), corev1.EventTypeWarning, "FailedToDrain", err.Error()) return errors.Wrap(err, "failed to drain node") } From 964b96638856472e32fb578ac9e782e37705ad9a Mon Sep 17 00:00:00 2001 From: Gal Zaidman Date: Wed, 1 Apr 2020 21:39:08 +0300 Subject: [PATCH 10/20] ovirt: disable tx checksum offload for workers This patch adds the workaround suggested on [1] to make nodeport work, instead of ethtool we use NM to apply the fix for each connction before it is up. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1794714 Signed-off-by: Gal Zaidman --- .../ovirt/files/ovirt-disable-tx-checksum-offload.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 templates/worker/00-worker/ovirt/files/ovirt-disable-tx-checksum-offload.yaml diff --git a/templates/worker/00-worker/ovirt/files/ovirt-disable-tx-checksum-offload.yaml b/templates/worker/00-worker/ovirt/files/ovirt-disable-tx-checksum-offload.yaml new file mode 100644 index 0000000000..912186717d --- /dev/null +++ b/templates/worker/00-worker/ovirt/files/ovirt-disable-tx-checksum-offload.yaml @@ -0,0 +1,10 @@ +filesystem: "root" +mode: 0744 +path: "/etc/NetworkManager/dispatcher.d/pre-up.d/disable-tx-checksum-offload.sh" +contents: + inline: | + #!/bin/bash + # This is a workaround for BZ#1794714 + if [[ -e /var/lib/cni/bin/openshift-sdn ]]; then + nmcli con modify ${CONNECTION_UUID} ethtool.feature-tx-checksum-ip-generic off; + fi From fb52d7d58c4144aac3b337bb43b63a34be367998 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Fri, 3 Apr 2020 09:42:28 +0200 Subject: [PATCH 11/20] OWNERS: add BZ component name Signed-off-by: Antonio Murdaca --- OWNERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/OWNERS b/OWNERS index 6dc5bb7adf..5e62da99da 100644 --- a/OWNERS +++ b/OWNERS @@ -10,3 +10,5 @@ approvers: - sinnykumari - yuqi-zhang - vrutkovs + +component: "Machine Config Operator" From ffdde2fb30d8e4c830ba1fcc014f384d1539b80c Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Fri, 3 Apr 2020 13:43:27 -0400 Subject: [PATCH 12/20] cri-o: set log level to info as it's difficult to debug with cri-o only set to log level error Signed-off-by: Peter Hunt --- .../master/01-master-container-runtime/_base/files/crio.yaml | 2 +- .../worker/01-worker-container-runtime/_base/files/crio.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/master/01-master-container-runtime/_base/files/crio.yaml b/templates/master/01-master-container-runtime/_base/files/crio.yaml index 66de159d06..2703a2d00e 100644 --- a/templates/master/01-master-container-runtime/_base/files/crio.yaml +++ b/templates/master/01-master-container-runtime/_base/files/crio.yaml @@ -155,7 +155,7 @@ contents: # Changes the verbosity of the logs based on the level it is set to. Options # are fatal, panic, error, warn, info, and debug. This option supports live # configuration reload. - log_level = "error" + log_level = "info" # The UID mappings for the user namespace of each container. A range is # specified in the form containerUID:HostUID:Size. Multiple ranges must be diff --git a/templates/worker/01-worker-container-runtime/_base/files/crio.yaml b/templates/worker/01-worker-container-runtime/_base/files/crio.yaml index 66de159d06..2703a2d00e 100644 --- a/templates/worker/01-worker-container-runtime/_base/files/crio.yaml +++ b/templates/worker/01-worker-container-runtime/_base/files/crio.yaml @@ -155,7 +155,7 @@ contents: # Changes the verbosity of the logs based on the level it is set to. Options # are fatal, panic, error, warn, info, and debug. This option supports live # configuration reload. - log_level = "error" + log_level = "info" # The UID mappings for the user namespace of each container. A range is # specified in the form containerUID:HostUID:Size. Multiple ranges must be From 9f8fd22b932b0a396b04252bfda2492a9184031d Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 3 Apr 2020 20:43:49 -0500 Subject: [PATCH 13/20] sdn: ignore new ovn-kubernetes OVS internal port names ovn-k8s-gw0 and ovn-k8s-mp0 --- templates/common/_base/files/nm-ignore-sdn.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/common/_base/files/nm-ignore-sdn.yaml b/templates/common/_base/files/nm-ignore-sdn.yaml index 52d4a18bb1..ffd13ac56f 100644 --- a/templates/common/_base/files/nm-ignore-sdn.yaml +++ b/templates/common/_base/files/nm-ignore-sdn.yaml @@ -5,5 +5,5 @@ contents: inline: | # ignore known SDN-managed devices [device] - match-device=interface-name:br-int;interface-name:br-local;interface-name:br-nexthop,interface-name:k8s-*;interface-name:tun0;interface-name:br0;driver:veth + match-device=interface-name:br-int;interface-name:br-local;interface-name:br-nexthop,interface-name:ovn-k8s-*,interface-name:k8s-*;interface-name:tun0;interface-name:br0;driver:veth managed=0 From 303810842837747601f29c58cfd6746ae148c799 Mon Sep 17 00:00:00 2001 From: Antoni Segura Puimedon Date: Mon, 6 Apr 2020 18:10:55 +0200 Subject: [PATCH 14/20] baremetal: static hostname to prevent DNS lookup When keepalived sets the VIP, it triggers a connection event in NetworkManager that, since the name we set from DHCP was transient it would do a reverse lookup on the connection address. Unfortunately, NM does not filter out the deprecated VIP for the reverse lookup and ends up overriding the hostname with DNS names that map to VIPs configured in the system. This fix is a workaround that on environments that have DHCP provide the hostname, will prevent the erroneous behavior of NM by setting DHCP provided FQDN addresses as static, which prevents NM from doing further address lookups. On hostname-less DHCP environments we'd want to hook to the hostname event in NM and make sure that the first one that does not map to a VIP is the one that gets set as static. Signed-off-by: Antoni Segura Puimedon --- .../baremetal/files/NetworkManager-resolv-prepender.yaml | 2 +- .../baremetal/files/NetworkManager-resolv-prepender.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/master/00-master/baremetal/files/NetworkManager-resolv-prepender.yaml b/templates/master/00-master/baremetal/files/NetworkManager-resolv-prepender.yaml index 82f0474437..94b05e623d 100644 --- a/templates/master/00-master/baremetal/files/NetworkManager-resolv-prepender.yaml +++ b/templates/master/00-master/baremetal/files/NetworkManager-resolv-prepender.yaml @@ -7,7 +7,7 @@ contents: IFACE=$1 STATUS=$2 # If $DHCP6_FQDN_FQDN is not empty and is not localhost.localdomain - [[ -n "$DHCP6_FQDN_FQDN" && "$DHCP6_FQDN_FQDN" != "localhost.localdomain" ]] && hostname $DHCP6_FQDN_FQDN + [[ -n "$DHCP6_FQDN_FQDN" && "$DHCP6_FQDN_FQDN" != "localhost.localdomain" && "$DHCP6_FQDN_FQDN" =~ "." ]] && hostnamectl set-hostname --static --transient $DHCP6_FQDN_FQDN case "$STATUS" in up|down|dhcp4-change|dhcp6-change) logger -s "NM resolv-prepender triggered by ${1} ${2}." diff --git a/templates/worker/00-worker/baremetal/files/NetworkManager-resolv-prepender.yaml b/templates/worker/00-worker/baremetal/files/NetworkManager-resolv-prepender.yaml index 79812b4041..bc221d9e13 100644 --- a/templates/worker/00-worker/baremetal/files/NetworkManager-resolv-prepender.yaml +++ b/templates/worker/00-worker/baremetal/files/NetworkManager-resolv-prepender.yaml @@ -7,7 +7,7 @@ contents: IFACE=$1 STATUS=$2 # If $DHCP6_FQDN_FQDN is not empty and is not localhost.localdomain - [[ -n "$DHCP6_FQDN_FQDN" && "$DHCP6_FQDN_FQDN" != "localhost.localdomain" ]] && hostname $DHCP6_FQDN_FQDN + [[ -n "$DHCP6_FQDN_FQDN" && "$DHCP6_FQDN_FQDN" != "localhost.localdomain" && "$DHCP6_FQDN_FQDN" =~ "." ]] && hostnamectl set-hostname --static --transient $DHCP6_FQDN_FQDN case "$STATUS" in up|down|dhcp4-change|dhcp6-change) logger -s "NM resolv-prepender triggered by ${1} ${2}." From bc9baeced3d7c920b80f72764a5f0c4412273506 Mon Sep 17 00:00:00 2001 From: Gal Zaidman Date: Mon, 6 Apr 2020 18:58:09 +0300 Subject: [PATCH 15/20] ovirt: fix disable tx checksum offload for workers On PR [1] we added a workaround for Bug [2], this fails when the worker starts for the first time since openshift-sdn is created only when the sdn pod is starting. Instead we will disable by default leave as is only when running with OVNkubernetes [1] On PR https://github.com/openshift/machine-config-operator/pull/1606, [2] https://bugzilla.redhat.com/show_bug.cgi?id=1794714 Signed-off-by: Gal Zaidman --- .../ovirt/files/ovirt-disable-tx-checksum-offload.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/worker/00-worker/ovirt/files/ovirt-disable-tx-checksum-offload.yaml b/templates/worker/00-worker/ovirt/files/ovirt-disable-tx-checksum-offload.yaml index 912186717d..dac73629aa 100644 --- a/templates/worker/00-worker/ovirt/files/ovirt-disable-tx-checksum-offload.yaml +++ b/templates/worker/00-worker/ovirt/files/ovirt-disable-tx-checksum-offload.yaml @@ -5,6 +5,6 @@ contents: inline: | #!/bin/bash # This is a workaround for BZ#1794714 - if [[ -e /var/lib/cni/bin/openshift-sdn ]]; then + if [[ ! -e /var/lib/cni/bin/ovn-k8s-cni-overlay ]]; then nmcli con modify ${CONNECTION_UUID} ethtool.feature-tx-checksum-ip-generic off; fi From 03c793850a2f25e2401d255290d3bd46443138aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Mon, 6 Apr 2020 17:21:01 +0200 Subject: [PATCH 16/20] OpenStack: disable tx checksum offload for workers This patch adds the workaround suggested on [1] to make nodeport work, instead of ethtool we use NM to apply the fix for each connction before it is up. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1794714 Brings the following ovirt fixes to openstack platform: - https://github.com/openshift/machine-config-operator/pull/1606 - https://github.com/openshift/machine-config-operator/pull/1621 --- .../files/openstack-disable-tx-checksum-offload.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 templates/worker/00-worker/openstack/files/openstack-disable-tx-checksum-offload.yaml diff --git a/templates/worker/00-worker/openstack/files/openstack-disable-tx-checksum-offload.yaml b/templates/worker/00-worker/openstack/files/openstack-disable-tx-checksum-offload.yaml new file mode 100644 index 0000000000..dac73629aa --- /dev/null +++ b/templates/worker/00-worker/openstack/files/openstack-disable-tx-checksum-offload.yaml @@ -0,0 +1,10 @@ +filesystem: "root" +mode: 0744 +path: "/etc/NetworkManager/dispatcher.d/pre-up.d/disable-tx-checksum-offload.sh" +contents: + inline: | + #!/bin/bash + # This is a workaround for BZ#1794714 + if [[ ! -e /var/lib/cni/bin/ovn-k8s-cni-overlay ]]; then + nmcli con modify ${CONNECTION_UUID} ethtool.feature-tx-checksum-ip-generic off; + fi From 49439a05e8d478382fc3082f91ae1e8af7f4c85b Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Tue, 7 Apr 2020 18:08:10 +0200 Subject: [PATCH 17/20] Makefile: Drop -mod=vendor flag from tests This is inferred by the golang-1.13 container now Remove now erroring copying of scripts and add workaround for k8s.io/code-generator not playing nice with go modules. --- Makefile | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 201369ca66..0e382e8254 100644 --- a/Makefile +++ b/Makefile @@ -19,11 +19,6 @@ export GOPROXY=https://proxy.golang.org GOTAGS = "containers_image_openpgp exclude_graphdriver_devicemapper exclude_graphdriver_btrfs containers_image_ostree_stub" -# grab the version from a dummy pkg in k8s.io/code-generator from vendor/modules.txt (read by go list) -versionPath=$(shell GO111MODULE=on go list -f {{.Dir}} k8s.io/code-generator/cmd/client-gen) -codegeneratorRoot=$(versionPath:/cmd/client-gen=) -codegeneratorTarget:=./vendor/k8s.io/code-generator - .PHONY: clean test test-unit test-e2e verify update install-tools # Remove build artifaces # Example: @@ -47,7 +42,7 @@ test: test-unit test-e2e # Unit tests only (no active cluster required) test-unit: - CGO_ENABLED=0 go test -mod=vendor -tags=$(GOTAGS) -count=1 -v ./cmd/... ./pkg/... ./lib/... + CGO_ENABLED=0 go test -tags=$(GOTAGS) -count=1 -v ./cmd/... ./pkg/... ./lib/... # Run the code generation tasks. # Example: @@ -60,14 +55,13 @@ go-deps: go mod tidy go mod vendor go mod verify - # go mod does not vendor in scripts so we need to get them manually... - @mkdir -p $(codegeneratorRoot) - @cp $(codegeneratorRoot)/generate-groups.sh $(codegeneratorTarget) && chmod +x $(codegeneratorTarget)/generate-groups.sh - @cp $(codegeneratorRoot)/generate-internal-groups.sh $(codegeneratorTarget) && chmod +x $(codegeneratorTarget)/generate-internal-groups.sh + # make scripts executable + chmod +x ./vendor/k8s.io/code-generator/generate-groups.sh + chmod +x ./vendor/k8s.io/code-generator/generate-internal-groups.sh install-tools: - GO111MODULE=on go build -o $(GOPATH)/bin/golangci-lint -mod=vendor ./vendor/github.com/golangci/golangci-lint/cmd/golangci-lint - GO111MODULE=on go build -o $(GOPATH)/bin/gosec -mod=vendor ./vendor/github.com/securego/gosec/cmd/gosec + GO111MODULE=on go build -o $(GOPATH)/bin/golangci-lint ./vendor/github.com/golangci/golangci-lint/cmd/golangci-lint + GO111MODULE=on go build -o $(GOPATH)/bin/gosec ./vendor/github.com/securego/gosec/cmd/gosec # Run verification steps # Example: @@ -77,6 +71,10 @@ verify: install-tools # Remove once https://github.com/golangci/golangci-lint/issues/597 is # addressed gosec -severity high --confidence medium -exclude G204 -quiet ./... + # Remove once code-generator plays nice with go modules, see + # https://github.com/kubernetes/kubernetes/issues/82531 and + # https://github.com/kubernetes/kubernetes/pull/85559 + pushd vendor/k8s.io/code-generator && go mod vendor && popd hack/verify-codegen.sh hack/verify-generated-bindata.sh @@ -105,6 +103,6 @@ Dockerfile.rhel7: Dockerfile Makefile (echo '# THIS FILE IS GENERATED FROM '$<' DO NOT EDIT' && \ sed -e s,org/openshift/release,org/ocp/builder, -e s,/openshift/origin-v4.0:base,/ocp/4.0:base, < $<) > $@.tmp && mv $@.tmp $@ -# This was copied from https://github.com/openshift/cluster-image-registry-operato +# This was copied from https://github.com/openshift/cluster-image-registry-operator test-e2e: - go test -mod=vendor -timeout 120m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/ + go test -timeout 120m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/ From 239447e6db692cecb6fe2ac94fffa5f40a28c577 Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Thu, 9 Apr 2020 02:11:03 +0200 Subject: [PATCH 18/20] Makefile: Cleanup after `make verify` Restore code-generator's go.mod file and remove vendor/k8s.io/code-generator/vendor directory after running verification in`make verify` --- Makefile | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 0e382e8254..092ce22922 100644 --- a/Makefile +++ b/Makefile @@ -71,12 +71,16 @@ verify: install-tools # Remove once https://github.com/golangci/golangci-lint/issues/597 is # addressed gosec -severity high --confidence medium -exclude G204 -quiet ./... - # Remove once code-generator plays nice with go modules, see + # Remove the vendor/k8s.io/code-generator vendor hack + # once code-generator plays nice with go modules, see # https://github.com/kubernetes/kubernetes/issues/82531 and # https://github.com/kubernetes/kubernetes/pull/85559 - pushd vendor/k8s.io/code-generator && go mod vendor && popd + pushd vendor/k8s.io/code-generator && cp go.mod go.mod.bak && go mod vendor && popd hack/verify-codegen.sh hack/verify-generated-bindata.sh + rm -f vendor/k8s.io/code-generator/go.mod + mv vendor/k8s.io/code-generator/go.mod.bak vendor/k8s.io/code-generator/go.mod + rm -rf vendor/k8s.io/code-generator/vendor # Template for defining build targets for binaries. define target_template = From c1ad8cde7c54eb01221fc0c9d6a543b8b4fc04e1 Mon Sep 17 00:00:00 2001 From: Urvashi Mohnani Date: Thu, 9 Apr 2020 14:25:45 -0400 Subject: [PATCH 19/20] Use a struct array instead of map when creating new ignitions A map doesn't guarantee order when we are creating new ignitions. When we update the image CR with blocked registries, the ctrcfg controller needs to update two files registries.conf and policy.json. Since we get an update from the image CR about every 20 mins, we compare the semantics to see if anything has changed. But since the order is not guaranteed, the controller might think that the semantics is not equal even if nothing in the data changed. Hence another MC is created, and everytime we get an update the MC applied to the nodes keeps flipping back and forth for the 2 possible orders causing the nodes to reboot a bunch of times. So move to using a struct array to ensure the order is always the same and we don't have two similar MCs being created. Signed-off-by: Urvashi Mohnani --- .../container_runtime_config_controller.go | 12 ++++++------ .../container-runtime-config/helpers.go | 18 +++++++++++++----- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/pkg/controller/container-runtime-config/container_runtime_config_controller.go b/pkg/controller/container-runtime-config/container_runtime_config_controller.go index aec210ffe7..1bd9caf3ea 100644 --- a/pkg/controller/container-runtime-config/container_runtime_config_controller.go +++ b/pkg/controller/container-runtime-config/container_runtime_config_controller.go @@ -552,9 +552,9 @@ func (ctrl *Controller) syncContainerRuntimeConfig(key string) error { } } - ctrRuntimeConfigIgn := createNewIgnition(map[string][]byte{ - storageConfigPath: storageTOML, - crioConfigPath: crioTOML, + ctrRuntimeConfigIgn := createNewIgnition([]ignitionConfig{ + {filePath: storageConfigPath, data: storageTOML}, + {filePath: crioConfigPath, data: crioTOML}, }) rawCtrRuntimeConfigIgn, err := json.Marshal(ctrRuntimeConfigIgn) if err != nil { @@ -766,9 +766,9 @@ func registriesConfigIgnition(templateDir string, controllerConfig *mcfgv1.Contr return nil, fmt.Errorf("could not update policy json with new changes: %v", err) } } - registriesIgn := createNewIgnition(map[string][]byte{ - registriesConfigPath: registriesTOML, - policyConfigPath: policyJSON, + registriesIgn := createNewIgnition([]ignitionConfig{ + {filePath: registriesConfigPath, data: registriesTOML}, + {filePath: policyConfigPath, data: policyJSON}, }) return ®istriesIgn, nil } diff --git a/pkg/controller/container-runtime-config/helpers.go b/pkg/controller/container-runtime-config/helpers.go index 79d072d337..3245b5471a 100644 --- a/pkg/controller/container-runtime-config/helpers.go +++ b/pkg/controller/container-runtime-config/helpers.go @@ -60,27 +60,35 @@ type tomlConfigCRIO struct { } `toml:"crio"` } +// ignitionConfig is a struct that holds the filepath and date of the various configs +// Using a struct array ensures that the order of the ignition files always stay the same +// ensuring that double MCs are not created due to a change in the order +type ignitionConfig struct { + filePath string + data []byte +} + type updateConfigFunc func(data []byte, internal *mcfgv1.ContainerRuntimeConfiguration) ([]byte, error) // createNewIgnition takes a map where the key is the path of the file, and the value is the // new data in the form of a byte array. The function returns the ignition config with the // updated data. -func createNewIgnition(configs map[string][]byte) ignTypes.Config { +func createNewIgnition(configs []ignitionConfig) ignTypes.Config { tempIgnConfig := ctrlcommon.NewIgnConfigSpecV3() mode := 0644 overwrite := true // Create ignitions - for filePath, data := range configs { + for _, ignConf := range configs { // If the file is not included, the data will be nil so skip over - if data == nil { + if ignConf.data == nil { continue } - configdu := dataurl.New(data, "text/plain") + configdu := dataurl.New(ignConf.data, "text/plain") configdu.Encoding = dataurl.EncodingASCII strConfigdu := configdu.String() configTempFile := ignTypes.File{ Node: ignTypes.Node{ - Path: filePath, + Path: ignConf.filePath, Overwrite: &overwrite, }, FileEmbedded1: ignTypes.FileEmbedded1{ From f4b8c5ef2274e285bad8547677865d31784e24bb Mon Sep 17 00:00:00 2001 From: Vadim Rutkovsky Date: Mon, 13 Apr 2020 10:50:16 +0200 Subject: [PATCH 20/20] make go-deps --- vendor/k8s.io/code-generator/generate-groups.sh | 0 vendor/k8s.io/code-generator/generate-internal-groups.sh | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 vendor/k8s.io/code-generator/generate-groups.sh mode change 100644 => 100755 vendor/k8s.io/code-generator/generate-internal-groups.sh diff --git a/vendor/k8s.io/code-generator/generate-groups.sh b/vendor/k8s.io/code-generator/generate-groups.sh old mode 100644 new mode 100755 diff --git a/vendor/k8s.io/code-generator/generate-internal-groups.sh b/vendor/k8s.io/code-generator/generate-internal-groups.sh old mode 100644 new mode 100755