Skip to content

Commit

Permalink
Support installing extensions and rework OS update
Browse files Browse the repository at this point in the history
Earlier to perform OS update we were pulling OS image
and mounting the content using podman. We were performing
OS update in host context because of selinux constraints
on mounted container.

Also, for rt-kernel switch we were pulling OS image again.
With coreos extensions support, we require extensions rpm
which is available in os container.

We tried different approach to solve problems
like minimizing container image pull, host/container
context switch, selinux permission on mounted
container and rpm-ostreed behavior.
See openshift#1850

Finally, using `oc image extract` and skopeo to inspect container
image solves our problems. With this we are also getting
rid of using mco-pivot systemd service to rebase OS.
  • Loading branch information
sinnykumari committed Jul 31, 2020
1 parent 2c9371f commit 6757381
Show file tree
Hide file tree
Showing 10 changed files with 245 additions and 227 deletions.
2 changes: 1 addition & 1 deletion cmd/machine-config-daemon/pivot.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func run(_ *cobra.Command, args []string) (retErr error) {

client := daemon.NewNodeUpdaterClient()

_, changed, err := client.PullAndRebase(container, keep)
changed, err := client.Rebase(container)
if err != nil {
return err
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,12 @@ spec:
description: Name is the name of the unit. This must be suffixed with a
valid unit type (e.g. 'thing.service')
type: string
extensions:
description: List of additional features that can be enabled on host
type: array
items:
type: string
nullable: true
fips:
description: FIPS controls FIPS mode
type: boolean
Expand Down
4 changes: 4 additions & 0 deletions lib/resourcemerge/machineconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@ func ensureMachineConfigSpec(modified *bool, existing *mcfgv1.MachineConfigSpec,
*modified = true
(*existing).FIPS = required.FIPS
}
if !equality.Semantic.DeepEqual(existing.Extensions, required.Extensions) {
*modified = true
(*existing).Extensions = required.Extensions
}
}

func ensureControllerConfigSpec(modified *bool, existing *mcfgv1.ControllerConfigSpec, required mcfgv1.ControllerConfigSpec) {
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/machineconfiguration.openshift.io/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ type MachineConfigSpec struct {

// +nullable
KernelArguments []string `json:"kernelArguments"`
Extensions []string `json:"extensions"`

FIPS bool `json:"fips"`
KernelType string `json:"kernelType"`
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 42 additions & 0 deletions pkg/controller/common/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*m
kargs = append(kargs, cfg.Spec.KernelArguments...)
}

extensions := []string{}
for _, cfg := range configs {
extensions = append(extensions, cfg.Spec.Extensions...)
}

// Ensure that kernel-devel extension is applied only with default kernel.
if kernelType != KernelTypeDefault {
if InSlice("kernel-devel", extensions) {
return nil, fmt.Errorf("Installing kernel-devel extension is not supported with kernelType: %s", kernelType)
}
}

return &mcfgv1.MachineConfig{
Spec: mcfgv1.MachineConfigSpec{
OSImageURL: osImageURL,
Expand All @@ -112,6 +124,7 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*m
},
FIPS: fips,
KernelType: kernelType,
Extensions: extensions,
},
}, nil
}
Expand Down Expand Up @@ -285,12 +298,41 @@ func ValidateIgnition(ignconfig interface{}) error {
}
}

// InSlice search for an element in slice and return true if found, otherwise return false
func InSlice(elem string, slice []string) bool {
for _, k := range slice {
if k == elem {
return true
}
}
return false
}

func validateExtensions(exts []string) error {
supportedExtensions := []string{"usbguard", "kernel-devel"}
invalidExts := []string{}
for _, ext := range exts {
if !InSlice(ext, supportedExtensions) {
invalidExts = append(invalidExts, ext)
}
}
if len(invalidExts) != 0 {
return fmt.Errorf("Invalid extensions found: %v", invalidExts)
}
return nil

}

// ValidateMachineConfig validates that given MachineConfig Spec is valid.
func ValidateMachineConfig(cfg mcfgv1.MachineConfigSpec) error {
if !(cfg.KernelType == "" || cfg.KernelType == KernelTypeDefault || cfg.KernelType == KernelTypeRealtime) {
return errors.Errorf("kernelType=%s is invalid", cfg.KernelType)
}

if err := validateExtensions(cfg.Extensions); err != nil {
return err
}

if cfg.Config.Raw != nil {
ignCfg, err := IgnParseWrapper(cfg.Config.Raw)
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,10 @@ func (dn *Daemon) checkStateOnFirstRun() error {
if !osMatch {
glog.Infof("Bootstrap pivot required to: %s", targetOSImageURL)
// This only returns on error
return dn.updateOSAndReboot(state.currentConfig)
if err := dn.updateOS(state.currentConfig); err != nil {
return err
}
return dn.finalizeAndReboot(state.currentConfig)
}
glog.Info("No bootstrap pivot required; unlinking bootstrap node annotations")

Expand Down
105 changes: 22 additions & 83 deletions pkg/daemon/rpm-ostree.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ import (
"github.com/golang/glog"
"github.com/opencontainers/go-digest"
"github.com/openshift/machine-config-operator/pkg/daemon/constants"
pivottypes "github.com/openshift/machine-config-operator/pkg/daemon/pivot/types"
pivotutils "github.com/openshift/machine-config-operator/pkg/daemon/pivot/utils"
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/util/uuid"
)

const (
Expand Down Expand Up @@ -65,7 +62,7 @@ type imageInspection struct {
type NodeUpdaterClient interface {
GetStatus() (string, error)
GetBootedOSImageURL() (string, string, error)
PullAndRebase(string, bool) (string, bool, error)
Rebase(string) (bool, error)
RunPivot(string) error
GetBootedDeployment() (*RpmOstreeDeployment, error)
}
Expand Down Expand Up @@ -133,15 +130,8 @@ func (r *RpmOstreeClient) GetBootedOSImageURL() (string, string, error) {
return osImageURL, bootedDeployment.Version, nil
}

// podmanRemove kills and removes a container
func podmanRemove(cid string) {
// Ignore errors here
exec.Command("podman", "kill", cid).Run()
exec.Command("podman", "rm", "-f", cid).Run()
}

// PullAndRebase potentially rebases system if not already rebased.
func (r *RpmOstreeClient) PullAndRebase(container string, keep bool) (imgid string, changed bool, err error) {
// Rebase potentially rebases system if not already rebased.
func (r *RpmOstreeClient) Rebase(container string) (changed bool, err error) {
defaultDeployment, err := r.GetBootedDeployment()
if err != nil {
return
Expand All @@ -164,87 +154,41 @@ func (r *RpmOstreeClient) PullAndRebase(container string, keep bool) (imgid stri
authArgs = append(authArgs, "--authfile", kubeletAuthFile)
}

// If we're passed a non-canonical image, resolve it to its sha256 now
isCanonicalForm := true
if _, err = getRefDigest(container); err != nil {
isCanonicalForm = false
// In non-canonical form, we pull unconditionally right now
args := []string{"pull", "-q"}
args = append(args, authArgs...)
args = append(args, container)
_, err = pivotutils.RunExt(numRetriesNetCommands, "podman", args...)
if err != nil {
return
}
} else {
if previousPivot != "" {
var targetMatched bool
targetMatched, err = compareOSImageURL(previousPivot, container)
if err != nil {
return
}
if targetMatched {
changed = false
return
}
}

// Pull the image
args := []string{"pull", "-q"}
args = append(args, authArgs...)
args = append(args, container)
_, err = pivotutils.RunExt(numRetriesNetCommands, "podman", args...)
if err != nil {
return
}
}
inspectArgs := []string{"inspect"}
inspectArgs = append(inspectArgs, authArgs...)

inspectArgs := []string{"inspect", "--type=image"}
inspectArgs = append(inspectArgs, fmt.Sprintf("%s", container))
// Map container image name with transport so that skopeo understands it
imageName := "docker://" + container
inspectArgs = append(inspectArgs, imageName)
var output []byte
output, err = runGetOut("podman", inspectArgs...)
output, err = runGetOut("skopeo", inspectArgs...)
if err != nil {
return
}
var imagedataArray []imageInspection
err = json.Unmarshal(output, &imagedataArray)
var imagedata imageInspection
err = json.Unmarshal(output, &imagedata)
if err != nil {
err = errors.Wrapf(err, "unmarshaling podman inspect")
err = errors.Wrapf(err, "unmarshaling skopeo inspect")
return
}
imagedata := imagedataArray[0]

// If we're passed a non-canonical image, resolve it to its sha256 now
isCanonicalForm := true
if _, err = getRefDigest(container); err != nil {
isCanonicalForm = false
}

var imgid string
if !isCanonicalForm {
imgid = imagedata.RepoDigests[0]
glog.Infof("Resolved to: %s", imgid)
} else {
imgid = container
}

containerName := pivottypes.PivotNamePrefix + string(uuid.NewUUID())

// `podman mount` wants a container, so let's make create a dummy one, but not run it
var cidBuf []byte
cidBuf, err = runGetOut("podman", "create", "--net=none", "--annotation=org.openshift.machineconfigoperator.pivot=true", "--name", containerName, imgid)
if err != nil {
return
}
defer func() {
// Kill our dummy container
podmanRemove(containerName)
}()

cid := strings.TrimSpace(string(cidBuf))
// Use the container ID to find its mount point
var mntBuf []byte
mntBuf, err = runGetOut("podman", "mount", cid)
if err != nil {
return
}
mnt := strings.TrimSpace(string(mntBuf))
repo := fmt.Sprintf("%s/srv/repo", mnt)
repo := fmt.Sprintf("%s/srv/repo", osImageContentDir)

// Now we need to figure out the commit to rebase to

// Commit label takes priority
ostreeCsum, ok := imagedata.Labels["com.coreos.ostree-commit"]
if ok {
Expand Down Expand Up @@ -281,6 +225,7 @@ func (r *RpmOstreeClient) PullAndRebase(container string, keep bool) (imgid stri

// This will be what will be displayed in `rpm-ostree status` as the "origin spec"
customURL := fmt.Sprintf("pivot://%s", imgid)
glog.Infof("Executing rebase from %s and checksum %s", customURL, ostreeCsum)

// RPM-OSTree can now directly slurp from the mounted container!
// https://github.com/projectatomic/rpm-ostree/pull/1732
Expand All @@ -292,12 +237,6 @@ func (r *RpmOstreeClient) PullAndRebase(container string, keep bool) (imgid stri
return
}

// By default, delete the image.
if !keep {
// Related: https://github.com/containers/libpod/issues/2234
exec.Command("podman", "rmi", imgid).Run()
}

changed = true
return
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/daemon/rpm-ostree_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ func (r RpmOstreeClientMock) RunPivot(string) error {
}

// PullAndRebase is a mock
func (r RpmOstreeClientMock) PullAndRebase(string, bool) (string, bool, error) {
return "", false, nil
func (r RpmOstreeClientMock) Rebase(string) (bool, error) {
return false, nil
}

func (r RpmOstreeClientMock) GetStatus() (string, error) {
Expand Down
Loading

0 comments on commit 6757381

Please sign in to comment.