Skip to content

Commit

Permalink
Switch from using hardcoded extensions to reading extensions.json
Browse files Browse the repository at this point in the history
Per issue 2445:

openshift#2445

Added extensions.json parsing and propagated a few errors since now
extension processing can fail.
Added a test for extensions.json parsing and filtering.

I don't know that I like this. If I understand the workflow right, this
feels like something that should maybe be done as part of a validation
step when extract the image so we only have to do it once and can check
the extensions against the container labels.

The package doesn't really "keep state" though (well, aside from the
filesystem), it just extracts the image where it's told so this would
require additional team conversations.
  • Loading branch information
jkyros committed Mar 15, 2021
1 parent 82868e6 commit 3a85532
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 17 deletions.
44 changes: 44 additions & 0 deletions pkg/daemon/extensions.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package daemon

import "io"
import "io/ioutil"
import "fmt"
import "encoding/json"

type SupportedExtensions struct {
Extensions map[string]Extension `json:"extensions"`
Repos []string `json:"repos"`
}

type Extension struct {
Packages []string `json:"packages"`
Architectures []string `json:"architectures,omitEmpty"`
Kind string `json:"kind"`
}

func parseSupportedExtensions(extensionsFile io.Reader) (map[string][]string, error) {

if rawExtensions, err := ioutil.ReadAll(extensionsFile); err != nil {
return nil, fmt.Errorf("failed to read extensions file %s : %s", extensionsFile, err)
} else {
var supportedExtensions *SupportedExtensions


if err := json.Unmarshal(rawExtensions, &supportedExtensions); err != nil {
return nil, fmt.Errorf("failed to unmarshal extensions file %s : %s", extensionsFile, err)
}

//https://github.com/openshift/machine-config-operator/issues/2445
//mco should only pay attention to the the os-extension kind
var extmap = make(map[string][]string)
for extkey, ext := range supportedExtensions.Extensions {
if ext.Kind == "os-extension" {
extmap[extkey] = supportedExtensions.Extensions[extkey].Packages
}
}


return extmap, nil
}
}

67 changes: 50 additions & 17 deletions pkg/daemon/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ const (
extensionsRepo = "/etc/yum.repos.d/coreos-extensions.repo"
osImageContentBaseDir = "/run/mco-machine-os-content/"

// extensionsFile is the json list of extensions from coreos assembler
extensionsFile = osImageContentBaseDir + "extensions/extensions.json"

// These are the actions for a node to take after applying config changes. (e.g. a new machineconfig is applied)
// "None" means no special action needs to be taken. A drain will still happen.
// This currently happens when ssh keys or pull secret (/var/lib/kubelet/config.json) is changed
Expand Down Expand Up @@ -1034,7 +1037,7 @@ func (dn *Daemon) updateKernelArguments(oldConfig, newConfig *mcfgv1.MachineConf
return err
}

func (dn *Daemon) generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineConfig) []string {
func (dn *Daemon) generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineConfig) ([]string, error) {
removed := []string{}
added := []string{}

Expand Down Expand Up @@ -1064,15 +1067,18 @@ func (dn *Daemon) generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineCon
extArgs := []string{"update"}

if dn.os.IsRHCOS() {
extensions := getSupportedExtensions()
for _, ext := range added {
for _, pkg := range extensions[ext] {
extArgs = append(extArgs, "--install", pkg)
if extensions, err := getSupportedExtensions(); err != nil {
return extArgs, err
} else {
for _, ext := range added {
for _, pkg := range extensions[ext] {
extArgs = append(extArgs, "--install", pkg)
}
}
}
for _, ext := range removed {
for _, pkg := range extensions[ext] {
extArgs = append(extArgs, "--uninstall", pkg)
for _, ext := range removed {
for _, pkg := range extensions[ext] {
extArgs = append(extArgs, "--uninstall", pkg)
}
}
}
}
Expand All @@ -1091,24 +1097,48 @@ func (dn *Daemon) generateExtensionsArgs(oldConfig, newConfig *mcfgv1.MachineCon
}
}

return extArgs
return extArgs, nil
}

// Returns list of extensions possible to install on a CoreOS based system.
func getSupportedExtensions() map[string][]string {
func getSupportedExtensions() (map[string][]string, error) {
// In future when list of extensions grow, it will make
// more sense to populate it in a dynamic way.

// These are RHCOS supported extensions.
// Each extension keeps a list of packages required to get enabled on host.
return map[string][]string{
"usbguard": {"usbguard"},
"kernel-devel": {"kernel-devel", "kernel-headers"},

//if the extensions file is there use it (post march 2021)
if _, err := os.Stat(extensionsFile); os.IsNotExist(err) {
extensionsFileReader, err := os.OpenFile(extensionsFile,os.O_RDONLY,0666)
if err != nil {
return map[string][]string{}, fmt.Errorf("Failed to open extensions file %s : %s", extensionsFile, err)
}


extmap, err := parseSupportedExtensions(extensionsFileReader)
if( err != nil ){
return extmap, err
}

return extmap, nil

//otherwise keep legacy hard coded behavior
} else {

return map[string][]string{
"usbguard": {"usbguard"},
"kernel-devel": {"kernel-devel", "kernel-headers"},
}, nil
}
}

func validateExtensions(exts []string) error {
supportedExtensions := getSupportedExtensions()
supportedExtensions, err := getSupportedExtensions()
if err != nil {
return err
}

invalidExts := []string{}
for _, ext := range exts {
if _, ok := supportedExtensions[ext]; !ok {
Expand Down Expand Up @@ -1138,9 +1168,12 @@ func (dn *Daemon) applyExtensions(oldConfig, newConfig *mcfgv1.MachineConfig) er
return err
}

args := dn.generateExtensionsArgs(oldConfig, newConfig)
args, err := dn.generateExtensionsArgs(oldConfig, newConfig)
if err != nil {
return err
}
glog.Infof("Applying extensions : %+q", args)
_, err := runGetOut("rpm-ostree", args...)
_, err = runGetOut("rpm-ostree", args...)

return err
}
Expand Down
69 changes: 69 additions & 0 deletions pkg/daemon/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"math/rand"
"reflect"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -647,3 +648,71 @@ func checkIrreconcilableResults(t *testing.T, key string, reconcilableError erro
t.Errorf("Different %s values should not be reconcilable.", key)
}
}

func TestParseExtensions(t *testing.T) {

supportedExtensionsReader := strings.NewReader(`
{
"extensions": {
"kernel-devel": {
"packages": [
"kernel-devel-4.18.0-240.15.1.el8_3",
"kernel-headers-4.18.0-240.15.1.el8_3"
],
"match-base-evr": "kernel",
"kind": "os-extension"
},
"kernel": {
"packages": [
"kernel-4.18.0-240.15.1.el8_3",
"kernel-core-4.18.0-240.15.1.el8_3",
"kernel-modules-4.18.0-240.15.1.el8_3",
"kernel-modules-extra-4.18.0-240.15.1.el8_3"
],
"match-base-evr": "kernel",
"kind": "development"
},
"kernel-rt": {
"packages": [
"kernel-rt-core",
"kernel-rt-kvm",
"kernel-rt-modules",
"kernel-rt-modules-extra",
"kernel-rt-devel"
],
"architectures": [
"x86_64"
],
"kind": "os-extension"
},
"usbguard": {
"packages": [
"usbguard"
],
"kind": "os-extension"
}
},
"repos": [
"rhel-8-nfv"
]
}`)

parsedExtensions := map[string][]string{
"kernel-devel": {"kernel-devel-4.18.0-240.15.1.el8_3", "kernel-headers-4.18.0-240.15.1.el8_3"},
"kernel-rt": {"kernel-rt-core", "kernel-rt-kvm", "kernel-rt-modules", "kernel-rt-modules-extra", "kernel-rt-devel"},
"usbguard": {"usbguard"},
}

//make sure we parse the extensions out right
supportedExtensions, err := parseSupportedExtensions(supportedExtensionsReader)
if err != nil {
t.Errorf("Expected a clean parse, got error %s", err)
}

if !reflect.DeepEqual(supportedExtensions, parsedExtensions) {
t.Error("Expected only os-extensions, list did not match")
for ext, pack := range supportedExtensions {
fmt.Printf("%s --> %s\n", ext, pack)
}
}
}

0 comments on commit 3a85532

Please sign in to comment.