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.

Sign up for GitHub

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

Jump to bottom

Ignition types.Config (v0s2) to runtime.RawExtension conversion #996

Merged
merged 4 commits into from
Apr 5, 2020
Merged

Ignition types.Config (v0s2) to runtime.RawExtension conversion #996

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b2ecad
Move MergeMachineConfigs func into controller/common pkg
LorbusChris Feb 13, 2020
14fc99f
test/e2e/utils_test.go: Use test/helpers.NewMachineConfig in createMC…
LorbusChris Mar 6, 2020
c3d970c
pkg/controller/common: Add KernelType constants
LorbusChris Mar 6, 2020
7a18cfb
Convert Ignition types.Config to runtime.RawExtension
LorbusChris Mar 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
71 changes: 41 additions & 30 deletions lib/resourceapply/machineconfig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/davecgh/go-spew/spew"
mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
"github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned/fake"
"github.com/openshift/machine-config-operator/test/helpers"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -175,12 +176,14 @@ func TestApplyMachineConfig(t *testing.T) {
input: &mcfgv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
Spec: mcfgv1.MachineConfigSpec{
Config: igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy",
}},
},
Config: runtime.RawExtension{
Raw: helpers.MarshalOrDie(&igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy",
}},
},
}),
},
},
},
Expand All @@ -199,12 +202,14 @@ func TestApplyMachineConfig(t *testing.T) {
expected := &mcfgv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{"extra": "leave-alone"}},
Spec: mcfgv1.MachineConfigSpec{
Config: igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy",
}},
},
Config: runtime.RawExtension{
Raw: helpers.MarshalOrDie(&igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy",
}},
},
}),
},
},
}
Expand All @@ -218,25 +223,29 @@ func TestApplyMachineConfig(t *testing.T) {
&mcfgv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{"extra": "leave-alone"}},
Spec: mcfgv1.MachineConfigSpec{
Config: igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy-prev",
}},
},
Config: runtime.RawExtension{
Raw: helpers.MarshalOrDie(&igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy-prev",
}},
},
}),
},
},
},
},
input: &mcfgv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "foo"},
Spec: mcfgv1.MachineConfigSpec{
Config: igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy",
}},
},
Config: runtime.RawExtension{
Raw: helpers.MarshalOrDie(&igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy",
}},
},
}),
},
},
},
Expand All @@ -255,12 +264,14 @@ func TestApplyMachineConfig(t *testing.T) {
expected := &mcfgv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Labels: map[string]string{"extra": "leave-alone"}},
Spec: mcfgv1.MachineConfigSpec{
Config: igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy",
}},
},
Config: runtime.RawExtension{
Raw: helpers.MarshalOrDie(&igntypes.Config{
Passwd: igntypes.Passwd{
Users: []igntypes.PasswdUser{{
HomeDir: "/home/dummy",
}},
},
}),
},
},
}
Expand Down
56 changes: 0 additions & 56 deletions pkg/apis/machineconfiguration.openshift.io/v1/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,67 +2,11 @@ package v1

import (
"fmt"
"sort"

ign "github.com/coreos/ignition/config/v2_2"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// MergeMachineConfigs combines multiple machineconfig objects into one object.
// It sorts all the configs in increasing order of their name.
// It uses the Ignition config from first object as base and appends all the rest.
// Kernel arguments are concatenated.
// It uses only the OSImageURL provided by the CVO and ignores any MC provided OSImageURL.
func MergeMachineConfigs(configs []*MachineConfig, osImageURL string) *MachineConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the pkg/apis name mean this subpackage is meant to be consumed by other repositories? If so, is it safe to remove this?

(I can’t see any user inside GitHub/openshift , at least).

Copy link
Member Author

Choose a reason for hiding this comment

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

The apis package will be moved into openshift/api eventually (hence moving out this func which needs ignition types). It's used by installer afaik

if len(configs) == 0 {
return nil
}
sort.Slice(configs, func(i, j int) bool { return configs[i].Name < configs[j].Name })

var fips bool
var kernelType string
outIgn := configs[0].Spec.Config
for idx := 1; idx < len(configs); idx++ {
// if any of the config has FIPS enabled, it'll be set
if configs[idx].Spec.FIPS {
fips = true
}
outIgn = ign.Append(outIgn, configs[idx].Spec.Config)
}

// sets the KernelType if specified in any of the MachineConfig
// Setting kerneType to realtime in any of MachineConfig takes priority
for _, cfg := range configs {
if cfg.Spec.KernelType == "realtime" {
kernelType = cfg.Spec.KernelType
break
} else if kernelType == "default" {
kernelType = cfg.Spec.KernelType
}
}

// If no MC sets kerneType, then set it to 'default' since that's what it is using
if kernelType == "" {
kernelType = "default"
}

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

return &MachineConfig{
Spec: MachineConfigSpec{
OSImageURL: osImageURL,
KernelArguments: kargs,
Config: outIgn,
FIPS: fips,
KernelType: kernelType,
},
}
}

// NewMachineConfigPoolCondition creates a new MachineConfigPool condition.
func NewMachineConfigPoolCondition(condType MachineConfigPoolConditionType, status corev1.ConditionStatus, reason, message string) *MachineConfigPoolCondition {
return &MachineConfigPoolCondition{
Expand Down
57 changes: 0 additions & 57 deletions pkg/apis/machineconfiguration.openshift.io/v1/machineconfig.deepcopy.go
View file
Open in desktop

This file was deleted.

5 changes: 2 additions & 3 deletions pkg/apis/machineconfiguration.openshift.io/v1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package v1

import (
igntypes "github.com/coreos/ignition/config/v2_2/types"
configv1 "github.com/openshift/api/config/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -153,7 +152,7 @@ type ControllerConfigList struct {
// +genclient
// +genclient:noStatus
// +genclient:nonNamespaced
// +k8s:deepcopy-gen=false
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

// MachineConfig defines the configuration for a machine
type MachineConfig struct {
Expand All @@ -169,7 +168,7 @@ type MachineConfigSpec struct {
// fetch the OS.
OSImageURL string `json:"osImageURL"`
// Config is a Ignition Config object.
Config igntypes.Config `json:"config"`
Config runtime.RawExtension `json:"config"`

// +nullable
KernelArguments []string `json:"kernelArguments"`
Expand Down
49 changes: 49 additions & 0 deletions pkg/apis/machineconfiguration.openshift.io/v1/zz_generated.deepcopy.go
View file
Open in desktop

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

7 changes: 5 additions & 2 deletions pkg/controller/bootstrap/bootstrap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"k8s.io/apimachinery/pkg/util/diff"

ign "github.com/coreos/ignition/config/v2_2"
igntypes "github.com/coreos/ignition/config/v2_2/types"
"github.com/openshift/machine-config-operator/lib/resourceread"
)
Expand Down Expand Up @@ -150,8 +151,10 @@ func TestBootstrapRun(t *testing.T) {

// Ensure that generated registries.conf corresponds to the testdata ImageContentSourcePolicy
var registriesConfig *igntypes.File
for i := range mc.Spec.Config.Storage.Files {
f := &mc.Spec.Config.Storage.Files[i]
ignCfg, _, err := ign.Parse(mc.Spec.Config.Raw)
require.NoError(t, err)
for i := range ignCfg.Storage.Files {
f := &ignCfg.Storage.Files[i]
if f.Path == "/etc/containers/registries.conf" {
registriesConfig = f
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/common/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@ const (

// ControllerConfigName is the name of the ControllerConfig object that controllers use
ControllerConfigName = "machine-config-controller"

// KernelTypeDefault denominates the default kernel type
KernelTypeDefault = "default"

// KernelTypeRealtime denominates the realtime kernel type
KernelTypeRealtime = "realtime"
)