Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support installing extensions shipped by RHCOS #1850

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions cmd/machine-config-daemon/mount_container.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package main

import (
"flag"
"fmt"
"os"

daemon "github.com/openshift/machine-config-operator/pkg/daemon"
"github.com/openshift/machine-config-operator/pkg/daemon/constants"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)

var mountContainer = &cobra.Command{
Use: "mount-container",
DisableFlagsInUseLine: true,
Short: "Pull and mount container",
Args: cobra.ExactArgs(1),
Run: executeMountContainer,
}

// init executes upon import
func init() {
rootCmd.AddCommand(mountContainer)
pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
}

func saveToFile(content, path string) error {
file, err := os.Create(path)
if err != nil {
file.Close()
return fmt.Errorf("Error creating file %s: %v", path, err)
}
defer file.Close()
if _, err := file.WriteString(content); err != nil {
return err
}
return nil

}

func runMountContainer(_ *cobra.Command, args []string) error {
flag.Set("logtostderr", "true")
flag.Parse()
var containerMntLoc, containerImage, containerName string
containerImage = args[0]

var err error
if containerMntLoc, containerName, err = daemon.MountOSContainer(containerImage); err != nil {
return err
}
// Save mounted container name and location into file for later to be used
// for OS rebase and applying extensions
if err := saveToFile(containerName, constants.MountedOSContainerName); err != nil {
return fmt.Errorf("Failed saving container name: %v", err)
}
if err := saveToFile(containerMntLoc, constants.MountedOSContainerLocation); err != nil {
return fmt.Errorf("Failed saving mounted container location: %v", err)
}

return nil

}

func executeMountContainer(cmd *cobra.Command, args []string) {
err := runMountContainer(cmd, args)
if err != nil {
fmt.Printf("error: %v\n", err)
os.Exit(1)
}
}
33 changes: 28 additions & 5 deletions cmd/machine-config-daemon/pivot.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/golang/glog"
daemon "github.com/openshift/machine-config-operator/pkg/daemon"
"github.com/openshift/machine-config-operator/pkg/daemon/constants"
"github.com/openshift/machine-config-operator/pkg/daemon/pivot/types"
errors "github.com/pkg/errors"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -214,8 +215,10 @@ func run(_ *cobra.Command, args []string) (retErr error) {
flag.Set("logtostderr", "true")
flag.Parse()

var container string
var containerName string
if fromEtcPullSpec || len(args) == 0 {
// In this case we will just rebase to OSImage provided in etcPivotFile.
// Extensions won't apply. This should be consistent with old behavior.
fromEtcPullSpec = true
data, err := ioutil.ReadFile(etcPivotFile)
if err != nil {
Expand All @@ -224,14 +227,34 @@ func run(_ *cobra.Command, args []string) (retErr error) {
}
return errors.Wrapf(err, "failed to read from %s", etcPivotFile)
}
container = strings.TrimSpace(string(data))
container := strings.TrimSpace(string(data))
unitName := "mco-mount-container"
glog.Infof("Pulling in image and mounting container on host via systemd-run unit=%s", unitName)
err = exec.Command("systemd-run", "--wait", "--collect", "--unit="+unitName,
"-E", "RPMOSTREE_CLIENT_ID=mco", constants.HostSelfBinary, "mount-container", container).Run()
if err != nil {
return errors.Wrapf(err, "failed to create extensions repo")
}
var containerName string
if containerName, err = daemon.ReadFromFile(constants.MountedOSContainerName); err != nil {
return err
}
Comment on lines +238 to +241
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be missing the point of this re-declaration of containerName.

Don't we want it to be available below, outside of the condition?

containerName, err = daemon.ReadFromFile(constants.MountedOSContainerName)
if err != nil {
    return err
}

Copy link
Member

@LorbusChris LorbusChris Jul 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meh.. I just realized there's a new PR 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will close this once we merge the other one.


defer func() {
// Ideally other than MCD pivot, OSContainer shouldn't be needed by others.
// With above assumption, we should be able to delete OSContainer image as well as associated container with force option
exec.Command("podman", "rm", containerName).Run()
exec.Command("podman", "rmi", container).Run()
glog.Infof("Deleted container %s and corresponding image %s", containerName, container)
}()

} else {
container = args[0]
containerName = args[0]
}

client := daemon.NewNodeUpdaterClient()

_, changed, err := client.PullAndRebase(container, keep)
changed, err := client.PerformRpmOSTreeOperations(containerName, keep)
if err != nil {
return err
}
Expand All @@ -257,7 +280,7 @@ func run(_ *cobra.Command, args []string) (retErr error) {
}

if !changed {
glog.Info("No changes; already at target oscontainer, no kernel args provided")
glog.Info("No changes; already at target oscontainer, no kernel args provided, no kernelType switch, no extensions applied")
}

return nil
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
19 changes: 19 additions & 0 deletions pkg/daemon/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,23 @@ const (
// "currentConfig" state. Create this file (empty contents is fine) if you wish the MCD
// to proceed and attempt to "reconcile" to the new "desiredConfig" state regardless.
MachineConfigDaemonForceFile = "/run/machine-config-daemon-force"

// NewMachineConfigPath contains all the data from the desired MachineConfig
// except the spec/Config. We use this information and compare with old MachineConfig
// during pivot to update OS to desired state.
NewMachineConfigPath = "/run/newMachineConfig.json"

// OldMachineConfigPath contains all the data from the MachineConfig from the current state
// except the spec/Config. We use this information and compare with new old MachienConfig
// during pivot to update OS to desired state.
OldMachineConfigPath = "/run/oldMachineConfig.json"

// MountedOSContainerName contains name of OSContainer which we have already pulled in, created and mounted
// while we created extensions repo. We are saving the already created container so that we can further
// perform OS rebase or install extensions
MountedOSContainerName = "/run/mountedOSContainerName"

// MountedOSContainerLocation contains the path of mounted container referenced in
// MountedOSContainer
MountedOSContainerLocation = "/run/mountedOSContainerLocation"
)
Loading