Skip to content

Commit

Permalink
OCPBUGS-10787: Persist static IP addressed NIC names from rhel8
Browse files Browse the repository at this point in the history
When upgrading rhel8 -> rhel9, some NIC names may change; e.g.
the `mlx5` driver started emitting port information.

This breaks static IP addressing which references those interfaces.

We're going to land code in nmstate to address this; see
nmstate/nmstate#2301

- For a few reasons; one is that this isn't an OCP specific problem
- nmstate is in Rust
- nmstate already parses network interface information
- Adding more glue into the MCO is undesirable

However...to solve the bootstrap problem here in that we don't
have this nmstate code on 4.12, add it to the MCD binary.
  • Loading branch information
cgwalters authored and openshift-cherrypick-robot committed Apr 6, 2023
1 parent 428c2e0 commit 333d938
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 19 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Expand Up @@ -20,7 +20,7 @@ RUN if [[ "${TAGS}" == "fcos" ]] || [[ "${TAGS}" == "scos" ]]; then \
# rewrite image names for fcos/scos
if [[ "${TAGS}" == "fcos" ]]; then sed -i 's/rhel-coreos/fedora-coreos/g' /manifests/*; \
elif [[ "${TAGS}" == "scos" ]]; then sed -i 's/rhel-coreos/centos-stream-coreos-9/g' /manifests/*; fi && \
if ! rpm -q util-linux; then yum install -y util-linux && yum clean all && rm -rf /var/cache/yum/*; fi
if ! rpm -q util-linux; then dnf install -y util-linux; fi && dnf -y install 'nmstate > 2.2' && dnf clean all && rm -rf /var/cache/dnf/*
COPY templates /etc/mcc/templates
ENTRYPOINT ["/usr/bin/machine-config-operator"]
LABEL io.openshift.release.operator true
13 changes: 12 additions & 1 deletion Dockerfile.rhel7
@@ -1,16 +1,27 @@
# THIS FILE IS GENERATED FROM Dockerfile DO NOT EDIT
FROM registry.ci.openshift.org/ocp/builder:rhel-8-golang-1.19-openshift-4.13 AS builder
ARG TAGS=""
WORKDIR /go/src/github.com/openshift/machine-config-operator
COPY . .
# FIXME once we can depend on a new enough host that supports globs for COPY,
# just use that. For now we work around this by copying a tarball.
RUN make install DESTDIR=./instroot && tar -C instroot -cf instroot.tar .

FROM registry.ci.openshift.org/ocp/4.13:base
ARG TAGS=""
COPY --from=builder /go/src/github.com/openshift/machine-config-operator/instroot.tar /tmp/instroot.tar
RUN cd / && tar xf /tmp/instroot.tar && rm -f /tmp/instroot.tar
COPY install /manifests
RUN if ! rpm -q util-linux; then yum install -y util-linux && yum clean all && rm -rf /var/cache/yum/*; fi

RUN if [[ "${TAGS}" == "fcos" ]] || [[ "${TAGS}" == "scos" ]]; then \
# comment out non-base/extensions image-references entirely for fcos/scos
sed -i '/- name: rhel-coreos-/,+3 s/^/#/' /manifests/image-references && \
# also remove extensions from the osimageurl configmap (if we don't, oc won't rewrite it, and the placeholder value will survive and get used)
sed -i '/baseOSExtensionsContainerImage:/ s/^/#/' /manifests/0000_80_machine-config-operator_05_osimageurl.yaml; fi && \
# rewrite image names for fcos/scos
if [[ "${TAGS}" == "fcos" ]]; then sed -i 's/rhel-coreos/fedora-coreos/g' /manifests/*; \
elif [[ "${TAGS}" == "scos" ]]; then sed -i 's/rhel-coreos/centos-stream-coreos-9/g' /manifests/*; fi && \
if ! rpm -q util-linux; then dnf install -y util-linux; fi && dnf -y install 'nmstate > 2.2' && dnf clean all && rm -rf /var/cache/dnf/*
COPY templates /etc/mcc/templates
ENTRYPOINT ["/usr/bin/machine-config-operator"]
LABEL io.openshift.release.operator true
17 changes: 17 additions & 0 deletions cmd/machine-config-daemon/firstboot_complete_machineconfig.go
Expand Up @@ -2,6 +2,7 @@ package main

import (
"flag"
"fmt"
"time"

"github.com/golang/glog"
Expand All @@ -18,9 +19,12 @@ var firstbootCompleteMachineconfig = &cobra.Command{
Run: executeFirstbootCompleteMachineConfig,
}

var maybePersistNics bool

// init executes upon import
func init() {
rootCmd.AddCommand(firstbootCompleteMachineconfig)
firstbootCompleteMachineconfig.PersistentFlags().BoolVar(&maybePersistNics, "maybe-persist-nics", false, "Run nmstatectl persist-nic-names")
pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
}

Expand All @@ -31,6 +35,19 @@ func runFirstBootCompleteMachineConfig(_ *cobra.Command, _ []string) error {
exitCh := make(chan error)
defer close(exitCh)

if maybePersistNics {
// If asked, before we try an OS update, persist NIC names (if applicable) so that
// we handle the reprovision case with old disk images and Ignition configs
// that provide static IP addresses.
if err := daemon.MaybePersistNetworkInterfaces("/rootfs"); err != nil {
return fmt.Errorf("failed to persist network interfaces: %w", err)
}
// We're done; this logic is distinct from the *non-containerized* /run/bin/machine-config-daemon
// for what I believe is historical reasons; we could shift everything to happening inside
// podman actually.
return nil
}

dn, err := daemon.New(exitCh)
if err != nil {
return err
Expand Down
18 changes: 1 addition & 17 deletions cmd/machine-config-daemon/start.go
Expand Up @@ -7,7 +7,6 @@ import (
"io"
"net/url"
"os"
"os/exec"
"path/filepath"
"syscall"

Expand Down Expand Up @@ -58,21 +57,6 @@ func init() {
startCmd.PersistentFlags().StringVar(&startOpts.promMetricsURL, "metrics-url", "127.0.0.1:8797", "URL for prometheus metrics listener")
}

// bindPodMounts ensures that the daemon can still see e.g. /run/secrets/kubernetes.io
// service account tokens after chrooting. This function must be called before chroot.
func bindPodMounts(rootMount string) error {
targetSecrets := filepath.Join(rootMount, "/run/secrets")
if err := os.MkdirAll(targetSecrets, 0o755); err != nil {
return err
}
// This will only affect our mount namespace, not the host
output, err := exec.Command("mount", "--rbind", "/run/secrets", targetSecrets).CombinedOutput()
if err != nil {
return fmt.Errorf("failed to mount /run/secrets to %s: %s: %w", targetSecrets, string(output), err)
}
return nil
}

func selfCopyToHost() error {
selfExecutableFd, err := os.Open("/proc/self/exe")
if err != nil {
Expand Down Expand Up @@ -117,7 +101,7 @@ func runStartCmd(cmd *cobra.Command, args []string) {
onceFromMode := startOpts.onceFrom != ""
if !onceFromMode {
// in the daemon case
if err := bindPodMounts(startOpts.rootMount); err != nil {
if err := daemon.PrepareNamespace(startOpts.rootMount); err != nil {
glog.Fatalf("Binding pod mounts: %+v", err)
}
}
Expand Down
72 changes: 72 additions & 0 deletions pkg/daemon/daemon.go
Expand Up @@ -156,6 +156,11 @@ const (
// also retrieve the pending config after a reboot
pendingStateMessageID = "machine-config-daemon-pending-state"

// originalContainerBin is the path at which we've stashed the MCD container's /usr/bin
// in the host namespace. We use this for executing any extra binaries we have in our
// container image.
originalContainerBin = "/run/machine-config-daemon-bin"

kubeletHealthzPollingInterval = 30 * time.Second
kubeletHealthzTimeout = 30 * time.Second

Expand Down Expand Up @@ -391,6 +396,32 @@ func (dn *Daemon) HypershiftConnect(
return nil
}

// PrepareNamespace is invoked before chrooting into the target root
func PrepareNamespace(target string) error {
// This contains the /run/secrets/kubernetes.io service account tokens that we still need
secretsMount := "/run/secrets"
targetSecrets := filepath.Join(target, secretsMount)
if err := os.MkdirAll(targetSecrets, 0o755); err != nil {
return err
}
// This will only affect our mount namespace, not the host
if err := runCmdSync("mount", "--rbind", secretsMount, targetSecrets); err != nil {
return fmt.Errorf("failed to mount %s to %s: %w", secretsMount, targetSecrets, err)
}

targetSavedBin := filepath.Join(target, originalContainerBin)
if err := os.MkdirAll(targetSavedBin, 0o755); err != nil {
return fmt.Errorf("failed to create %s: %w", targetSavedBin, err)
}

usrbin := "/usr/bin"
if err := runCmdSync("mount", "--rbind", usrbin, targetSavedBin); err != nil {
return fmt.Errorf("failed to mount %s to %s: %w", usrbin, targetSavedBin, err)
}

return nil
}

// writer implements io.Writer interface as a pass-through for klog.
type writer struct {
logFunc func(args ...interface{})
Expand Down Expand Up @@ -564,6 +595,9 @@ func (dn *Daemon) syncNode(key string) error {
if err := removeIgnitionArtifacts(); err != nil {
return err
}
if err := MaybePersistNetworkInterfaces("/"); err != nil {
return err
}
if err := dn.checkStateOnFirstRun(); err != nil {
return err
}
Expand Down Expand Up @@ -1520,6 +1554,44 @@ func removeIgnitionArtifacts() error {
return nil
}

// MaybePersistNetworkInterfaces runs if the host is RHEL8, which can happen
// when scaling up older bootimages and targeting 4.13+ (rhel9). In this case,
// we may want to pin NIC interface names that reference static IP addresses.
// More information in https://issues.redhat.com/browse/OCPBUGS-10787
func MaybePersistNetworkInterfaces(osRoot string) error {
hostos, err := osrelease.GetHostRunningOSFromRoot(osRoot)
if err != nil {
return fmt.Errorf("checking operating system: %w", err)
}

nmstateBinary := "/usr/bin/nmstatectl"
// If we're already chrooted into the host / in the MCD case, then we
// need to find the binary in our saved copy of /usr/bin from the host.
if osRoot == "/" {
nmstateBinary = filepath.Join(originalContainerBin, "nmstatectl")
}

cmd := exec.Command(nmstateBinary, "persist-nic-names", "--root", osRoot)

if hostos.IsEL8() {
glog.Info("Persisting NIC names for RHEL8 host system")
} else {
// If we're not on RHEL, just output the current state. We don't strictly need to do
// this but it will greatly help debugging and validation.
cmd.Args = append(cmd.Args, "--inspect")
}

// nmstate always logs to stderr, so we need to capture/forward that too
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
glog.Infof("Running: %s", strings.Join(cmd.Args, " "))
if err := cmd.Run(); err != nil {
return fmt.Errorf("failed to run nmstatectl: %w", err)
}

return nil
}

// When we move from RHCOS 8 -> RHCOS 9, the SSH keys do not get written to the
// new location before the node reboots into RHCOS 9 because:
//
Expand Down
12 changes: 12 additions & 0 deletions pkg/daemon/osrelease/osrelease.go
Expand Up @@ -62,6 +62,11 @@ func (os OperatingSystem) IsEL() bool {
return os.id == rhcos || os.id == scos
}

// IsEL8 is true if the OS is RHCOS 8 or SCOS 8
func (os OperatingSystem) IsEL8() bool {
return os.IsEL() && strings.HasPrefix(os.version, "8.") || os.version == "8"
}

// IsEL9 is true if the OS is RHCOS 9 or SCOS 9
func (os OperatingSystem) IsEL9() bool {
return os.IsEL() && strings.HasPrefix(os.version, "9.") || os.version == "9"
Expand Down Expand Up @@ -105,6 +110,13 @@ func GetHostRunningOS() (OperatingSystem, error) {
return newOperatingSystem(EtcOSReleasePath, LibOSReleasePath)
}

// GetHostRunningOSFromRoot reads the os-release data from an alternative root
func GetHostRunningOSFromRoot(root string) (OperatingSystem, error) {
etcPath := filepath.Join(root, EtcOSReleasePath)
libPath := filepath.Join(root, LibOSReleasePath)
return newOperatingSystem(etcPath, libPath)
}

// Generates the OperatingSystem data from strings which contain the desired
// content. Mostly useful for testing purposes.
func LoadOSRelease(etcOSReleaseContent, libOSReleaseContent string) (OperatingSystem, error) {
Expand Down
Expand Up @@ -15,6 +15,10 @@ contents: |
RemainAfterExit=yes
# Disable existing repos (if any) so that OS extensions would use embedded RPMs only
ExecStartPre=-/usr/bin/sh -c "sed -i 's/enabled=1/enabled=0/' /etc/yum.repos.d/*.repo"
# Run this via podman because we want to use the nmstatectl binary in our container
ExecStart=/usr/bin/podman run --rm --privileged --net=host -v /:/rootfs --entrypoint machine-config-daemon '{{ .Images.machineConfigOperator }}' firstboot-complete-machineconfig --maybe-persist-nics
# This one was copied out to the host...but actually I'm not sure why we didn't
# run it via podman to start
ExecStart=/run/bin/machine-config-daemon firstboot-complete-machineconfig
{{if .Proxy -}}
EnvironmentFile=/etc/mco/proxy.env
Expand Down

0 comments on commit 333d938

Please sign in to comment.