diff --git a/pkg/controller/build/buildrequest/buildrequest.go b/pkg/controller/build/buildrequest/buildrequest.go index 5bd5b300e3..5d997a5522 100644 --- a/pkg/controller/build/buildrequest/buildrequest.go +++ b/pkg/controller/build/buildrequest/buildrequest.go @@ -335,8 +335,8 @@ func (br buildRequestImpl) renderContainerfile() (string, error) { MachineOSBuild: br.opts.MachineOSBuild, MachineOSConfig: br.opts.MachineOSConfig, UserContainerfile: br.userContainerfile, - BaseOSImage: br.opts.OSImageURLConfig.BaseOSContainerImage, - ExtensionsImage: br.opts.OSImageURLConfig.BaseOSExtensionsContainerImage, + BaseOSImage: br.opts.MachineConfig.Spec.OSImageURL, + ExtensionsImage: br.opts.MachineConfig.Spec.BaseOSExtensionsContainerImage, ExtensionsPackages: extPkgs, KernelType: kernelType, KernelPackages: kernelPackages, @@ -671,7 +671,7 @@ func (br buildRequestImpl) toBuildahPod() *corev1.Pod { // us to avoid parsing log files. Name: "create-digest-configmap", Command: append(command, digestCMScript), - Image: br.opts.OSImageURLConfig.BaseOSContainerImage, + Image: br.opts.MachineConfig.Spec.OSImageURL, Env: env, ImagePullPolicy: corev1.PullAlways, SecurityContext: securityContext, diff --git a/pkg/controller/build/buildrequest/buildrequest_test.go b/pkg/controller/build/buildrequest/buildrequest_test.go index 7939084747..ef9671348f 100644 --- a/pkg/controller/build/buildrequest/buildrequest_test.go +++ b/pkg/controller/build/buildrequest/buildrequest_test.go @@ -42,13 +42,11 @@ func TestBuildRequestInvalidExtensions(t *testing.T) { func TestBuildRequest(t *testing.T) { t.Parallel() - osImageURLConfig := fixtures.OSImageURLConfig() - expectedContents := func() []string { return []string{ - fmt.Sprintf("FROM %s AS extract", osImageURLConfig.BaseOSContainerImage), - fmt.Sprintf("FROM %s AS configs", osImageURLConfig.BaseOSContainerImage), - fmt.Sprintf("LABEL baseOSContainerImage=%s", osImageURLConfig.BaseOSContainerImage), + fmt.Sprintf("FROM %s AS extract", fixtures.BaseOSContainerImage), + fmt.Sprintf("FROM %s AS configs", fixtures.BaseOSContainerImage), + fmt.Sprintf("LABEL baseOSContainerImage=%s", fixtures.BaseOSContainerImage), } } @@ -66,7 +64,7 @@ func TestBuildRequest(t *testing.T) { return opts }, expectedContainerfileContents: append(expectedContents(), []string{ - fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage), + fmt.Sprintf("RUN --mount=type=bind,from=%s", fixtures.BaseOSExtensionsContainerImage), `extensions="usbguard"`, }...), }, @@ -78,7 +76,7 @@ func TestBuildRequest(t *testing.T) { return opts }, expectedContainerfileContents: append(expectedContents(), []string{ - fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSExtensionsContainerImage), + fmt.Sprintf("RUN --mount=type=bind,from=%s", fixtures.BaseOSExtensionsContainerImage), `extensions="krb5-workstation libkadm5 usbguard"`, }...), }, @@ -86,12 +84,12 @@ func TestBuildRequest(t *testing.T) { name: "Missing extensions image and extensions", optsFunc: func() BuildRequestOpts { opts := getBuildRequestOpts() - opts.OSImageURLConfig.BaseOSExtensionsContainerImage = "" + opts.MachineConfig.Spec.BaseOSExtensionsContainerImage = "" opts.MachineConfig.Spec.Extensions = []string{"usbguard"} return opts }, unexpectedContainerfileContents: []string{ - fmt.Sprintf("RUN --mount=type=bind,from=%s", osImageURLConfig.BaseOSContainerImage), + fmt.Sprintf("RUN --mount=type=bind,from=%s", fixtures.BaseOSContainerImage), "extensions=\"usbguard\"", }, }, @@ -235,7 +233,7 @@ func assertBuildJobIsCorrect(t *testing.T, buildJob *batchv1.Job, opts BuildRequ assert.Equal(t, buildJob.Spec.Template.Spec.InitContainers[0].Image, mcoImagePullspec) expectedPullspecs := []string{ "base-os-image-from-machineosconfig", - fixtures.OSImageURLConfig().BaseOSContainerImage, + fixtures.BaseOSContainerImage, } assert.Contains(t, expectedPullspecs, buildJob.Spec.Template.Spec.Containers[0].Image) @@ -312,7 +310,7 @@ RUN rpm-ostree install && \ newSecret := `{"auths":` + legacySecret + `}` return BuildRequestOpts{ - MachineConfig: &mcfgv1.MachineConfig{}, + MachineConfig: fixtures.NewObjectsForTest("worker").RenderedMachineConfig, MachineOSConfig: layeredObjects.MachineOSConfigBuilder.MachineOSConfig(), MachineOSBuild: layeredObjects.MachineOSBuildBuilder.MachineOSBuild(), Images: &ctrlcommon.Images{ @@ -320,7 +318,6 @@ RUN rpm-ostree install && \ MachineConfigOperator: mcoImagePullspec, }, }, - OSImageURLConfig: fixtures.OSImageURLConfig(), BaseImagePullSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "base-image-pull-secret", diff --git a/pkg/controller/build/buildrequest/buildrequestopts.go b/pkg/controller/build/buildrequest/buildrequestopts.go index 3cb2e4a511..61f0584f48 100644 --- a/pkg/controller/build/buildrequest/buildrequestopts.go +++ b/pkg/controller/build/buildrequest/buildrequestopts.go @@ -23,11 +23,10 @@ import ( // Holds all of the options used to produce a BuildRequest. type BuildRequestOpts struct { //nolint:revive // This name is fine. - MachineOSConfig *mcfgv1.MachineOSConfig - MachineOSBuild *mcfgv1.MachineOSBuild - MachineConfig *mcfgv1.MachineConfig - Images *ctrlcommon.Images - OSImageURLConfig *ctrlcommon.OSImageURLConfig + MachineOSConfig *mcfgv1.MachineOSConfig + MachineOSBuild *mcfgv1.MachineOSBuild + MachineConfig *mcfgv1.MachineConfig + Images *ctrlcommon.Images BaseImagePullSecret *corev1.Secret FinalImagePushSecret *corev1.Secret @@ -100,10 +99,6 @@ func newBuildRequestOptsFromAPI(ctx context.Context, kubeclient clientset.Interf return nil, fmt.Errorf("expected images to not be nil") } - if opts.OSImageURLConfig == nil { - return nil, fmt.Errorf("expected osimageurlconfig to not be nil") - } - if opts.BaseImagePullSecret == nil { return nil, fmt.Errorf("expected base image pull secret to not be nil") } @@ -176,11 +171,6 @@ func (o *optsGetter) getOpts(ctx context.Context, mosb *mcfgv1.MachineOSBuild, m return nil, fmt.Errorf("could not get images.json config: %w", err) } - osImageURLConfig, err := ctrlcommon.GetOSImageURLConfig(ctx, o.kubeclient) - if err != nil { - return nil, fmt.Errorf("could not get osImageURL config: %w", err) - } - var baseImagePullSecretName string // Check if a base image pull secret was provided opts.hasUserDefinedBaseImagePullSecret = mosc.Spec.BaseImagePullSecret != nil @@ -214,7 +204,6 @@ func (o *optsGetter) getOpts(ctx context.Context, mosb *mcfgv1.MachineOSBuild, m opts.Images = imagesConfig opts.MachineConfig = mc - opts.OSImageURLConfig = osImageURLConfig opts.BaseImagePullSecret = baseImagePullSecret opts.FinalImagePushSecret = finalImagePushSecret opts.MachineOSConfig = mosc.DeepCopy() diff --git a/pkg/controller/build/buildrequest/buildrequestopts_test.go b/pkg/controller/build/buildrequest/buildrequestopts_test.go index e1a6e81625..8e80cb13f8 100644 --- a/pkg/controller/build/buildrequest/buildrequestopts_test.go +++ b/pkg/controller/build/buildrequest/buildrequestopts_test.go @@ -148,7 +148,6 @@ func TestBuildRequestOpts(t *testing.T) { assert.NotNil(t, brOpts.MachineOSConfig) assert.NotNil(t, brOpts.MachineOSBuild) assert.NotNil(t, brOpts.Images) - assert.NotNil(t, brOpts.OSImageURLConfig) assert.NotNil(t, brOpts.BaseImagePullSecret) assert.NotNil(t, brOpts.FinalImagePushSecret) }) diff --git a/pkg/controller/build/buildrequest/machineosbuild.go b/pkg/controller/build/buildrequest/machineosbuild.go index 3ac3f398da..29948e3b9c 100644 --- a/pkg/controller/build/buildrequest/machineosbuild.go +++ b/pkg/controller/build/buildrequest/machineosbuild.go @@ -1,10 +1,11 @@ package buildrequest import ( - "context" + //nolint:gosec "crypto/md5" "fmt" + "strings" "github.com/distribution/reference" "github.com/ghodss/yaml" @@ -13,7 +14,6 @@ import ( "github.com/openshift/machine-config-operator/pkg/controller/build/utils" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - clientset "k8s.io/client-go/kubernetes" ) // This is the same salt / pattern from pkg/controller/render/hash.go @@ -32,13 +32,17 @@ var ( // Holds the objects that are used to construct a MachineOSBuild with a hashed // name. type MachineOSBuildOpts struct { + MachineConfig *mcfgv1.MachineConfig MachineOSConfig *mcfgv1.MachineOSConfig MachineConfigPool *mcfgv1.MachineConfigPool - OSImageURLConfig *ctrlcommon.OSImageURLConfig } // Validates that the required options are provided. func (m *MachineOSBuildOpts) validateForHash() error { + if err := m.validateMachineConfig(); err != nil { + return fmt.Errorf("machineconfig failed validation: %w", err) + } + if m.MachineOSConfig == nil { return fmt.Errorf("missing required MachineOSConfig") } @@ -51,8 +55,30 @@ func (m *MachineOSBuildOpts) validateForHash() error { return fmt.Errorf("name mismatch, MachineConfigPool has %q, MachineOSConfig has %q", m.MachineConfigPool.Name, m.MachineOSConfig.Spec.MachineConfigPool.Name) } - if m.OSImageURLConfig == nil { - return fmt.Errorf("misssing OSImageURLConfig") + return nil +} + +// Validates that a MachineConfig has the necessary metadata for generating a +// MachineOSBuild. +func (m *MachineOSBuildOpts) validateMachineConfig() error { + if m.MachineConfig == nil { + return fmt.Errorf("missing required MachineConfig") + } + + if !strings.HasPrefix(m.MachineConfig.Name, "rendered-") { + return fmt.Errorf("machineconfig %q is not a rendered MachineConfig", m.MachineConfig.Name) + } + + requiredAnnos := []string{ctrlcommon.ReleaseImageVersionAnnotationKey, ctrlcommon.GeneratedByControllerVersionAnnotationKey} + for _, anno := range requiredAnnos { + val, ok := m.MachineConfig.Annotations[anno] + if !ok { + return fmt.Errorf("missing annotation %q on MachineConfig %q", anno, m.MachineConfig.Name) + } + + if val == "" { + return fmt.Errorf("empty annotation %q value on MachineConfig %q", anno, m.MachineConfig.Name) + } } return nil @@ -60,6 +86,24 @@ func (m *MachineOSBuildOpts) validateForHash() error { // Creates a list of objects that are consumed by the SHA256 hash. func (m *MachineOSBuildOpts) objectsForHash() []interface{} { + // Represents a private version of the OSImageURLConfig struct to keep the + // hashed name generation stable regardless of the input source. This means + // that we can eventually remove the OSImageURLConfig struct. + type osImageURLConfig struct { + BaseOSContainerImage string + BaseOSExtensionsContainerImage string + OSImageURL string + ReleaseVersion string + } + + cfg := osImageURLConfig{ + BaseOSContainerImage: m.MachineConfig.Spec.OSImageURL, + BaseOSExtensionsContainerImage: m.MachineConfig.Spec.BaseOSExtensionsContainerImage, + // This value is purposely left empty because the ConfigMap does not actually + // populate this value. However, we want the hashing to be stable. + OSImageURL: "", + ReleaseVersion: m.MachineConfig.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey], + } // The objects considered for hashing described inline: out := []interface{}{ @@ -70,8 +114,8 @@ func (m *MachineOSBuildOpts) objectsForHash() []interface{} { m.MachineConfigPool.Spec.Configuration, // The MachineOSConfig Spec field. m.MachineOSConfig.Spec, - // The complete OSImageURLConfig object. - m.OSImageURLConfig, + // The complete osImageURLConfig object. + cfg, } return out @@ -118,23 +162,6 @@ func (m *MachineOSBuildOpts) getHashedName() (string, error) { return fmt.Sprintf("%x", hasher.Sum(nil)), nil } -// Constructs the MachineOSBuildOpts by retrieving the OSImageURLConfig from -// the API server. -func NewMachineOSBuildOpts(ctx context.Context, kubeclient clientset.Interface, mosc *mcfgv1.MachineOSConfig, mcp *mcfgv1.MachineConfigPool) (MachineOSBuildOpts, error) { - // TODO: Consider an implementation that uses listers instead of API clients - // just to cut down on API server traffic. - osImageURLs, err := ctrlcommon.GetOSImageURLConfig(ctx, kubeclient) - if err != nil { - return MachineOSBuildOpts{}, fmt.Errorf("could not get OSImageURLConfig: %w", err) - } - - return MachineOSBuildOpts{ - MachineOSConfig: mosc, - MachineConfigPool: mcp, - OSImageURLConfig: osImageURLs, - }, nil -} - // Constructs a new MachineOSBuild object or panics trying. Useful for testing // scenarios. func NewMachineOSBuildOrDie(opts MachineOSBuildOpts) *mcfgv1.MachineOSBuild { @@ -147,30 +174,6 @@ func NewMachineOSBuildOrDie(opts MachineOSBuildOpts) *mcfgv1.MachineOSBuild { return mosb } -// Retrieves the MachineOSBuildOpts from the API and constructs a new -// MachineOSBuild object or panics trying. Useful for testing scenarios. -func NewMachineOSBuildFromAPIOrDie(ctx context.Context, kubeclient clientset.Interface, mosc *mcfgv1.MachineOSConfig, mcp *mcfgv1.MachineConfigPool) *mcfgv1.MachineOSBuild { - mosb, err := NewMachineOSBuildFromAPI(ctx, kubeclient, mosc, mcp) - - if err != nil { - panic(err) - } - - return mosb -} - -// Retrieves the MachineOSBuildOpts from the API and constructs a new -// MachineOSBuild object. -func NewMachineOSBuildFromAPI(ctx context.Context, kubeclient clientset.Interface, mosc *mcfgv1.MachineOSConfig, mcp *mcfgv1.MachineConfigPool) (*mcfgv1.MachineOSBuild, error) { - opts, err := NewMachineOSBuildOpts(ctx, kubeclient, mosc, mcp) - - if err != nil { - return nil, fmt.Errorf("could not get MachineOSBuildOpts: %w", err) - } - - return NewMachineOSBuild(opts) -} - // Constructs a new MachineOSBuild object with all of the labels, the tagged // image pushpsec, and a hashed name. func NewMachineOSBuild(opts MachineOSBuildOpts) (*mcfgv1.MachineOSBuild, error) { diff --git a/pkg/controller/build/buildrequest/machineosbuild_test.go b/pkg/controller/build/buildrequest/machineosbuild_test.go index 5235d71332..d59dc18dd8 100644 --- a/pkg/controller/build/buildrequest/machineosbuild_test.go +++ b/pkg/controller/build/buildrequest/machineosbuild_test.go @@ -19,6 +19,10 @@ func TestMachineOSBuild(t *testing.T) { poolName := "worker" + getMachineConfig := func() *mcfgv1.MachineConfig { + return fixtures.NewObjectsForTest(poolName).RenderedMachineConfig + } + getMachineOSConfig := func() *mcfgv1.MachineOSConfig { return testhelpers.NewMachineOSConfigBuilder(poolName).WithMachineConfigPool(poolName).MachineOSConfig() } @@ -28,7 +32,7 @@ func TestMachineOSBuild(t *testing.T) { } // Some of the test cases expect the hash name to be the same. This is that hash value. - expectedCommonHashName := "worker-55592464e51104dcc274a300565fec9e" + expectedCommonHashName := "worker-699e6be74658adcb3ff2b48f32cd1584" testCases := []struct { name string @@ -40,6 +44,7 @@ func TestMachineOSBuild(t *testing.T) { name: "Missing MachineConfigPool", errExpected: true, opts: MachineOSBuildOpts{ + MachineConfig: getMachineConfig(), MachineOSConfig: getMachineOSConfig(), }, }, @@ -47,6 +52,7 @@ func TestMachineOSBuild(t *testing.T) { name: "Missing MachineOSConfig", errExpected: true, opts: MachineOSBuildOpts{ + MachineConfig: getMachineConfig(), MachineConfigPool: getMachineConfigPool(), }, }, @@ -54,30 +60,27 @@ func TestMachineOSBuild(t *testing.T) { name: "Mismatched MachineConfigPool name and MachineOSConfig", errExpected: true, opts: MachineOSBuildOpts{ + MachineConfig: getMachineConfig(), MachineOSConfig: testhelpers.NewMachineOSConfigBuilder("worker").WithMachineConfigPool("other-pool").MachineOSConfig(), MachineConfigPool: getMachineConfigPool(), }, }, { - name: "Only MachineOSConfig and MachineConfigPool", - expectedName: "worker-6782c5fc52947bc8fa6d105c9fe62b7d", - errExpected: true, + name: "Missing MachineConfig", + errExpected: true, opts: MachineOSBuildOpts{ MachineOSConfig: getMachineOSConfig(), MachineConfigPool: getMachineConfigPool(), }, }, - // These cases ensure that the hashed name remains stable regardless of - // which source of truth is used for the base OS image, extensions image, - // and / or release version. In these cases, the source of truth can either - // be the value from the MachineOSConfig or the OSImageURLConfig struct. + // These cases ensure that the hashed name remains stable. { - name: "All values from OSImageURLConfig", + name: "All values present", expectedName: expectedCommonHashName, opts: MachineOSBuildOpts{ + MachineConfig: getMachineConfig(), MachineOSConfig: getMachineOSConfig(), MachineConfigPool: getMachineConfigPool(), - OSImageURLConfig: fixtures.OSImageURLConfig(), }, }, // These cases ensure that pausing the MachineConfigPool does not affect the hash. @@ -85,18 +88,18 @@ func TestMachineOSBuild(t *testing.T) { name: "Unpaused MachineConfigPool", expectedName: expectedCommonHashName, opts: MachineOSBuildOpts{ + MachineConfig: getMachineConfig(), MachineOSConfig: getMachineOSConfig(), MachineConfigPool: getMachineConfigPool(), - OSImageURLConfig: fixtures.OSImageURLConfig(), }, }, { name: "Paused MachineConfigPool", expectedName: expectedCommonHashName, opts: MachineOSBuildOpts{ + MachineConfig: getMachineConfig(), MachineOSConfig: getMachineOSConfig(), MachineConfigPool: testhelpers.NewMachineConfigPoolBuilder(poolName).WithPaused().MachineConfigPool(), - OSImageURLConfig: fixtures.OSImageURLConfig(), }, }, } @@ -139,12 +142,14 @@ func TestMachineOSBuild(t *testing.T) { func TestMachineOSBuildLabelConsistency(t *testing.T) { t.Parallel() - obj := fixtures.NewObjectsForTest("worker") + poolName := "worker" + + obj := fixtures.NewObjectsForTest(poolName) mosb, err := NewMachineOSBuild(MachineOSBuildOpts{ + MachineConfig: fixtures.NewObjectsForTest(poolName).RenderedMachineConfig, MachineConfigPool: obj.MachineConfigPool, MachineOSConfig: obj.MachineOSConfig, - OSImageURLConfig: fixtures.OSImageURLConfig(), }) assert.NoError(t, err) diff --git a/pkg/controller/build/fixtures/objects.go b/pkg/controller/build/fixtures/objects.go index f4459213cd..1e566413ed 100644 --- a/pkg/controller/build/fixtures/objects.go +++ b/pkg/controller/build/fixtures/objects.go @@ -21,10 +21,11 @@ const ( // Provides consistently instantiated objects for use in a given test. type ObjectsForTest struct { - MachineConfigPool *mcfgv1.MachineConfigPool - MachineConfigs []*mcfgv1.MachineConfig - MachineOSConfig *mcfgv1.MachineOSConfig - MachineOSBuild *mcfgv1.MachineOSBuild + MachineConfigPool *mcfgv1.MachineConfigPool + MachineConfigs []*mcfgv1.MachineConfig + MachineOSConfig *mcfgv1.MachineOSConfig + MachineOSBuild *mcfgv1.MachineOSBuild + RenderedMachineConfig *mcfgv1.MachineConfig } // Provides the builders to create consistently instantiated objects for use in @@ -38,11 +39,14 @@ type ObjectBuildersForTest struct { func (o *ObjectBuildersForTest) ToObjectsForTest() ObjectsForTest { mcp := o.MachineConfigPoolBuilder.MachineConfigPool() + mcs, renderedMC := newMachineConfigsFromPool(mcp) + return ObjectsForTest{ - MachineConfigPool: mcp, - MachineConfigs: newMachineConfigsFromPool(mcp), - MachineOSConfig: o.MachineOSConfigBuilder.MachineOSConfig(), - MachineOSBuild: o.MachineOSBuildBuilder.MachineOSBuild(), + MachineConfigPool: mcp, + MachineConfigs: mcs, + MachineOSConfig: o.MachineOSConfigBuilder.MachineOSConfig(), + MachineOSBuild: o.MachineOSBuildBuilder.MachineOSBuild(), + RenderedMachineConfig: renderedMC, } } @@ -51,7 +55,7 @@ func (o *ObjectBuildersForTest) ToObjectsForTest() ObjectsForTest { func (o *ObjectsForTest) ToRuntimeObjects() []runtime.Object { out := []runtime.Object{o.MachineConfigPool} - for _, item := range o.MachineConfigs { + for _, item := range append(o.MachineConfigs, o.RenderedMachineConfig) { out = append(out, item) } @@ -162,7 +166,7 @@ func defaultKubeObjects() []runtime.Object { } // Generates MachineConfigs from the given MachineConfigPool for insertion. -func newMachineConfigsFromPool(mcp *mcfgv1.MachineConfigPool) []*mcfgv1.MachineConfig { +func newMachineConfigsFromPool(mcp *mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfig, *mcfgv1.MachineConfig) { files := []ign3types.File{} out := []*mcfgv1.MachineConfig{} @@ -186,17 +190,21 @@ func newMachineConfigsFromPool(mcp *mcfgv1.MachineConfigPool) []*mcfgv1.MachineC []ign3types.File{file})) } - // Create a rendered MachineConfig to accompany our MachineConfigPool. - out = append(out, testhelpers.NewMachineConfig( + renderedMC := testhelpers.NewMachineConfig( mcp.Spec.Configuration.Name, - map[string]string{ - ctrlcommon.GeneratedByControllerVersionAnnotationKey: "version-number", - "machineconfiguration.openshift.io/role": mcp.Name, - }, + map[string]string{}, "", - files)) + files) - return out + renderedMC.Annotations = map[string]string{ + ctrlcommon.ReleaseImageVersionAnnotationKey: ReleaseVersion, + ctrlcommon.GeneratedByControllerVersionAnnotationKey: "controller-version", + } + + renderedMC.Spec.OSImageURL = BaseOSContainerImage + renderedMC.Spec.BaseOSExtensionsContainerImage = BaseOSExtensionsContainerImage + + return out, renderedMC } // Gets an example machine-config-operator-images ConfigMap. diff --git a/pkg/controller/build/osbuildcontroller_test.go b/pkg/controller/build/osbuildcontroller_test.go index ef9c76dd91..9f334e5423 100644 --- a/pkg/controller/build/osbuildcontroller_test.go +++ b/pkg/controller/build/osbuildcontroller_test.go @@ -3,12 +3,10 @@ package build import ( "context" "fmt" - "path/filepath" "testing" "time" "github.com/containers/image/v5/types" - ign3types "github.com/coreos/ignition/v2/config/v3_5/types" "github.com/opencontainers/go-digest" mcfgv1 "github.com/openshift/api/machineconfiguration/v1" fakeclientimagev1 "github.com/openshift/client-go/image/clientset/versioned/fake" @@ -83,7 +81,7 @@ func TestOSBuildControllerDeletesRunningBuildBeforeStartingANewOne(t *testing.T) t.Run("MachineOSConfig change", func(t *testing.T) { - kubeclient, mcfgclient, _, _, mosc, initialMosb, mcp, kubeassert, _ := setupOSBuildControllerForTestWithRunningBuild(ctx, t, poolName) + kubeclient, mcfgclient, _, _, mosc, initialMosb, mcp, kubeassert, lobj, _ := setupOSBuildControllerForTestWithRunningBuild(ctx, t, poolName) // Now that the build is in the running state, we update the MachineOSConfig. apiMosc := testhelpers.SetContainerfileContentsOnMachineOSConfig(ctx, t, mcfgclient, mosc, "FROM configs AS final\nRUN echo 'helloworld' > /etc/helloworld") @@ -91,7 +89,13 @@ func TestOSBuildControllerDeletesRunningBuildBeforeStartingANewOne(t *testing.T) apiMosc, err := mcfgclient.MachineconfigurationV1().MachineOSConfigs().Update(ctx, apiMosc, metav1.UpdateOptions{}) require.NoError(t, err) - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, apiMosc, mcp) + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: lobj.RenderedMachineConfig, + MachineOSConfig: apiMosc, + MachineConfigPool: mcp, + }) + + // mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, apiMosc, mcp) buildJobName := utils.GetBuildJobName(mosb) // After creating the new MachineOSConfig, a MachineOSBuild should be created. @@ -114,11 +118,15 @@ func TestOSBuildControllerDeletesRunningBuildBeforeStartingANewOne(t *testing.T) t.Run("MachineConfig change", func(t *testing.T) { - kubeclient, mcfgclient, _, _, mosc, initialMosb, mcp, kubeassert, _ := setupOSBuildControllerForTestWithRunningBuild(ctx, t, poolName) + _, mcfgclient, _, _, mosc, initialMosb, mcp, kubeassert, _, _ := setupOSBuildControllerForTestWithRunningBuild(ctx, t, poolName) - apiMCP := insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, mosc.Spec.MachineConfigPool.Name, "rendered-worker-2") + apiMCP, apiMC := insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, mosc.Spec.MachineConfigPool.Name, "rendered-worker-2") - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, mosc, apiMCP) + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: apiMC, + MachineOSConfig: mosc, + MachineConfigPool: apiMCP, + }) buildJobName := utils.GetBuildJobName(mosb) @@ -145,7 +153,7 @@ func TestOSBuildControllerLeavesSuccessfulBuildAlone(t *testing.T) { poolName := "worker" - kubeclient, mcfgclient, _, _, firstMosc, firstMosb, mcp, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName) + kubeclient, mcfgclient, _, _, firstMosc, firstMosb, mcp, lobj, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName) // Ensures that we have detected the first build. isMachineOSBuildReachedExpectedCount(ctx, t, mcfgclient, firstMosc, 1) @@ -156,7 +164,12 @@ func TestOSBuildControllerLeavesSuccessfulBuildAlone(t *testing.T) { newMosc := testhelpers.SetContainerfileContentsOnMachineOSConfig(ctx, t, mcfgclient, mosc, containerfileContents) // Compute the new MachineOSBuild. - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, newMosc, mcp) + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: lobj.RenderedMachineConfig, + MachineOSConfig: newMosc, + MachineConfigPool: mcp, + }) + // mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, newMosc, mcp) // Ensure that the MachineOSBuild exists. kubeassert.MachineOSBuildExists(mosb) @@ -215,7 +228,7 @@ func TestOSBuildControllerFailure(t *testing.T) { t.Run("Failed build objects remain", func(t *testing.T) { - _, _, _, _, _, failedMosb, _, kubeassert := setupOSBuildControllerForTestWithFailedBuild(ctx, t, poolName) + _, _, _, _, _, failedMosb, _, kubeassert, _ := setupOSBuildControllerForTestWithFailedBuild(ctx, t, poolName) // Ensure that even after failure, the build objects remain. assertBuildObjectsAreCreated(ctx, t, kubeassert, failedMosb) @@ -223,13 +236,17 @@ func TestOSBuildControllerFailure(t *testing.T) { t.Run("MachineOSConfig change clears failed build", func(t *testing.T) { - kubeclient, mcfgclient, _, _, mosc, failedMosb, mcp, kubeassert := setupOSBuildControllerForTestWithFailedBuild(ctx, t, poolName) + kubeclient, mcfgclient, _, _, mosc, failedMosb, mcp, kubeassert, lobj := setupOSBuildControllerForTestWithFailedBuild(ctx, t, poolName) // Modify the MachineOSConfig to start a new build. newMosc := testhelpers.SetContainerfileContentsOnMachineOSConfig(ctx, t, mcfgclient, mosc, "FROM configs AS final\nRUN echo 'helloworld' > /etc/helloworld") // Compute the new MachineOSBuild. - newMosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, newMosc, mcp) + newMosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: lobj.RenderedMachineConfig, + MachineOSConfig: newMosc, + MachineConfigPool: mcp, + }) // Ensure that the MachineOSBuild exists. kubeassert.MachineOSBuildExists(newMosb) @@ -247,11 +264,16 @@ func TestOSBuildControllerFailure(t *testing.T) { t.Run("MachineConfig change clears failed build", func(t *testing.T) { - kubeclient, mcfgclient, _, _, mosc, failedMosb, mcp, kubeassert := setupOSBuildControllerForTestWithFailedBuild(ctx, t, poolName) + _, mcfgclient, _, _, mosc, failedMosb, mcp, kubeassert, _ := setupOSBuildControllerForTestWithFailedBuild(ctx, t, poolName) + + apiMCP, apiMC := insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, mosc.Spec.MachineConfigPool.Name, "rendered-worker-2") - apiMCP := insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, mosc.Spec.MachineConfigPool.Name, "rendered-worker-2") + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: apiMC, + MachineOSConfig: mosc, + MachineConfigPool: apiMCP, + }) - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, mosc, apiMCP) buildJobName := utils.GetBuildJobName(mosb) // After updating the MachineConfigPool, a new MachineOSBuild should get created. kubeassert.MachineOSBuildExists(mosb, "New MachineOSBuild for MachineConfigPool %q update for MachineOSConfig %q never gets created", mcp.Name, mosc.Name) @@ -285,7 +307,7 @@ func TestOSBuildController(t *testing.T) { t.Run("MachineOSConfig changes creates a new MachineOSBuild", func(t *testing.T) { - kubeclient, mcfgclient, _, _, mosc, _, _, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName) + kubeclient, mcfgclient, _, _, mosc, _, _, lobj, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName) // Update the BuildInputs section on the MachineOSConfig and verify that a // new MachineOSBuild is produced from it. We'll do this 10 times. @@ -295,7 +317,12 @@ func TestOSBuildController(t *testing.T) { apiMCP, err := mcfgclient.MachineconfigurationV1().MachineConfigPools().Get(ctx, apiMosc.Spec.MachineConfigPool.Name, metav1.GetOptions{}) require.NoError(t, err) - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, apiMosc, apiMCP) + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: lobj.RenderedMachineConfig, + MachineOSConfig: apiMosc, + MachineConfigPool: apiMCP, + }) + buildJobName := utils.GetBuildJobName(mosb) // After creating the new MachineOSConfig, a MachineOSBuild should be created. kubeassert.MachineOSBuildExists(mosb, "MachineOSBuild not created for MachineOSConfig %s change, iteration %d", mosc.Name, i) @@ -326,17 +353,22 @@ func TestOSBuildController(t *testing.T) { t.Run("MachineConfig changes creates a new MachineOSBuild", func(t *testing.T) { - kubeclient, mcfgclient, _, _, mosc, _, mcp, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName) + kubeclient, mcfgclient, _, _, mosc, _, mcp, _, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName) // Update the rendered MachineConfig on the MachineConfigPool and verify that a new MachineOSBuild is produced. We'll do this 10 times. for i := 0; i <= 5; i++ { apiMosc, err := mcfgclient.MachineconfigurationV1().MachineOSConfigs().Get(ctx, mosc.Name, metav1.GetOptions{}) require.NoError(t, err) - apiMCP := insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, mosc.Spec.MachineConfigPool.Name, getConfigNameForPool(i+2)) + apiMCP, apiMC := insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, mosc.Spec.MachineConfigPool.Name, getConfigNameForPool(i+2)) time.Sleep(time.Millisecond * 200) - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, apiMosc, apiMCP) + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: apiMC, + MachineOSConfig: apiMosc, + MachineConfigPool: apiMCP, + }) + buildJobName := utils.GetBuildJobName(mosb) // After updating the MachineConfigPool, a new MachineOSBuild should get created. kubeassert.MachineOSBuildExists(mosb, "New MachineOSBuild for MachineConfigPool %q update for MachineOSConfig %q never gets created", mcp.Name, mosc.Name) @@ -372,7 +404,7 @@ func TestOSBuildControllerBuildFailedDoesNotCascade(t *testing.T) { faultyMC := "rendered-undesiredFaultyMC" // Create a MOSC to enable OCL and let it produce a new MOSB in Running State - _, mcfgclient, _, _, mosc, mosb, mcp, _, ctrl := setupOSBuildControllerForTestWithRunningBuild(ctx, t, poolName) + _, mcfgclient, _, _, mosc, mosb, mcp, _, _, ctrl := setupOSBuildControllerForTestWithRunningBuild(ctx, t, poolName) assertMachineOSConfigGetsCurrentBuildAnnotation(ctx, t, mcfgclient, mosc, mosb) found := func(item *mcfgv1.MachineOSBuild, list []mcfgv1.MachineOSBuild) bool { @@ -447,16 +479,20 @@ func TestOSBuildControllerReconcilesMachineConfigPoolsAfterRestart(t *testing.T) // Gets an OSBuildController with a running job. ctrlCtx, ctrlCtxCancel := context.WithCancel(ctx) t.Cleanup(ctrlCtxCancel) - kubeclient, mcfgclient, imageclient, routeclient, mosc, firstMosb, mcp, kubeassert, _ := setupOSBuildControllerForTestWithRunningBuild(ctrlCtx, t, poolName) + kubeclient, mcfgclient, imageclient, routeclient, mosc, firstMosb, _, kubeassert, _, _ := setupOSBuildControllerForTestWithRunningBuild(ctrlCtx, t, poolName) // Stop the OSBuildController. ctrlCtxCancel() // Create a MachineConfigPool change. - mcp = insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, poolName, "rendered-worker-2") + apiMCP, apiMC := insertNewRenderedMachineConfigAndUpdatePool(ctx, t, mcfgclient, poolName, "rendered-worker-2") // Get the name of the second MachineOSBuild object. - secondMosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, mosc, mcp) + secondMosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: apiMC, + MachineOSConfig: mosc, + MachineConfigPool: apiMCP, + }) // Ensure that everything still exists. kubeassert = kubeassert.Eventually().WithContext(ctx) @@ -537,7 +573,12 @@ func TestOSBuildControllerReconcilesJobsAfterRestart(t *testing.T) { _, err := mcfgclient.MachineconfigurationV1().MachineOSConfigs().Create(ctx, mosc, metav1.CreateOptions{}) require.NoError(t, err) - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, mosc, mcp) + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: lobj.RenderedMachineConfig, + MachineOSConfig: mosc, + MachineConfigPool: mcp, + }) + apiMosb, err := mcfgclient.MachineconfigurationV1().MachineOSBuilds().Create(ctx, mosb, metav1.CreateOptions{}) require.NoError(t, err) @@ -644,7 +685,7 @@ func setupOSBuildControllerForTest(ctx context.Context, t *testing.T) (*fakecore return kubeclient, mcfgclient, imageclient, routeclient, kubeassert, lobj, ctrl } -func setupOSBuildControllerForTestWithBuild(ctx context.Context, t *testing.T, poolName string) (*fakecorev1client.Clientset, *fakeclientmachineconfigv1.Clientset, *fakeclientimagev1.Clientset, *fakeclientroutev1.Clientset, *mcfgv1.MachineOSConfig, *mcfgv1.MachineOSBuild, *mcfgv1.MachineConfigPool, *testhelpers.Assertions, *OSBuildController) { +func setupOSBuildControllerForTestWithBuild(ctx context.Context, t *testing.T, poolName string) (*fakecorev1client.Clientset, *fakeclientmachineconfigv1.Clientset, *fakeclientimagev1.Clientset, *fakeclientroutev1.Clientset, *mcfgv1.MachineOSConfig, *mcfgv1.MachineOSBuild, *mcfgv1.MachineConfigPool, *testhelpers.Assertions, *fixtures.ObjectsForTest, *OSBuildController) { kubeclient, mcfgclient, imageclient, routeclient, kubeassert, lobj, ctrl := setupOSBuildControllerForTest(ctx, t) mcp := lobj.MachineConfigPool @@ -654,15 +695,19 @@ func setupOSBuildControllerForTestWithBuild(ctx context.Context, t *testing.T, p _, err := mcfgclient.MachineconfigurationV1().MachineOSConfigs().Create(ctx, mosc, metav1.CreateOptions{}) require.NoError(t, err) - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, kubeclient, mosc, mcp) + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: lobj.RenderedMachineConfig, + MachineOSConfig: mosc, + MachineConfigPool: mcp, + }) - return kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert.WithPollInterval(time.Millisecond * 10).WithContext(ctx).Eventually(), ctrl + return kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert.WithPollInterval(time.Millisecond * 10).WithContext(ctx).Eventually(), lobj, ctrl } -func setupOSBuildControllerForTestWithRunningBuild(ctx context.Context, t *testing.T, poolName string) (*fakecorev1client.Clientset, *fakeclientmachineconfigv1.Clientset, *fakeclientimagev1.Clientset, *fakeclientroutev1.Clientset, *mcfgv1.MachineOSConfig, *mcfgv1.MachineOSBuild, *mcfgv1.MachineConfigPool, *testhelpers.Assertions, *OSBuildController) { +func setupOSBuildControllerForTestWithRunningBuild(ctx context.Context, t *testing.T, poolName string) (*fakecorev1client.Clientset, *fakeclientmachineconfigv1.Clientset, *fakeclientimagev1.Clientset, *fakeclientroutev1.Clientset, *mcfgv1.MachineOSConfig, *mcfgv1.MachineOSBuild, *mcfgv1.MachineConfigPool, *testhelpers.Assertions, *fixtures.ObjectsForTest, *OSBuildController) { t.Helper() - kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, ctrl := setupOSBuildControllerForTestWithBuild(ctx, t, poolName) + kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, lobj, ctrl := setupOSBuildControllerForTestWithBuild(ctx, t, poolName) time.Sleep(time.Millisecond * 200) initialBuildJobName := utils.GetBuildJobName(mosb) @@ -678,13 +723,13 @@ func setupOSBuildControllerForTestWithRunningBuild(ctx context.Context, t *testi // The MachineOSBuild should be running. kubeassert.Eventually().WithContext(ctx).MachineOSBuildIsRunning(mosb, "Expected the MachineOSBuild %s status to be running", mosb.Name) - return kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, ctrl + return kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, lobj, ctrl } -func setupOSBuildControllerForTestWithSuccessfulBuild(ctx context.Context, t *testing.T, poolName string) (*fakecorev1client.Clientset, *fakeclientmachineconfigv1.Clientset, *fakeclientimagev1.Clientset, *fakeclientroutev1.Clientset, *mcfgv1.MachineOSConfig, *mcfgv1.MachineOSBuild, *mcfgv1.MachineConfigPool, *testhelpers.Assertions) { +func setupOSBuildControllerForTestWithSuccessfulBuild(ctx context.Context, t *testing.T, poolName string) (*fakecorev1client.Clientset, *fakeclientmachineconfigv1.Clientset, *fakeclientimagev1.Clientset, *fakeclientroutev1.Clientset, *mcfgv1.MachineOSConfig, *mcfgv1.MachineOSBuild, *mcfgv1.MachineConfigPool, *fixtures.ObjectsForTest, *testhelpers.Assertions) { t.Helper() - kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, _ := setupOSBuildControllerForTestWithRunningBuild(ctx, t, poolName) + kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, lobj, _ := setupOSBuildControllerForTestWithRunningBuild(ctx, t, poolName) time.Sleep(time.Millisecond * 200) kubeassert.MachineOSBuildExists(mosb) kubeassert.JobExists(utils.GetBuildJobName(mosb)) @@ -693,13 +738,13 @@ func setupOSBuildControllerForTestWithSuccessfulBuild(ctx context.Context, t *te kubeassert.MachineOSBuildIsSuccessful(mosb) kubeassert.JobDoesNotExist(utils.GetBuildJobName(mosb)) - return kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert + return kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, lobj, kubeassert } -func setupOSBuildControllerForTestWithFailedBuild(ctx context.Context, t *testing.T, poolName string) (*fakecorev1client.Clientset, *fakeclientmachineconfigv1.Clientset, *fakeclientimagev1.Clientset, *fakeclientroutev1.Clientset, *mcfgv1.MachineOSConfig, *mcfgv1.MachineOSBuild, *mcfgv1.MachineConfigPool, *testhelpers.Assertions) { +func setupOSBuildControllerForTestWithFailedBuild(ctx context.Context, t *testing.T, poolName string) (*fakecorev1client.Clientset, *fakeclientmachineconfigv1.Clientset, *fakeclientimagev1.Clientset, *fakeclientroutev1.Clientset, *mcfgv1.MachineOSConfig, *mcfgv1.MachineOSBuild, *mcfgv1.MachineConfigPool, *testhelpers.Assertions, *fixtures.ObjectsForTest) { t.Helper() - kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, _ := setupOSBuildControllerForTestWithBuild(ctx, t, poolName) + kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, lobj, _ := setupOSBuildControllerForTestWithBuild(ctx, t, poolName) initialBuildJobName := utils.GetBuildJobName(mosb) @@ -712,67 +757,56 @@ func setupOSBuildControllerForTestWithFailedBuild(ctx context.Context, t *testin // The MachineOSBuild should be running. kubeassert.MachineOSBuildIsRunning(mosb, "Expected the MachineOSBuild %s status to be running", mosb.Name) - return kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert + return kubeclient, mcfgclient, imageclient, routeclient, mosc, mosb, mcp, kubeassert, lobj } -func insertNewRenderedMachineConfigAndUpdatePool(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, poolName, renderedName string) *mcfgv1.MachineConfigPool { +func insertNewRenderedMachineConfigAndUpdatePool(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, poolName, renderedName string) (*mcfgv1.MachineConfigPool, *mcfgv1.MachineConfig) { mcp, err := mcfgclient.MachineconfigurationV1().MachineConfigPools().Get(ctx, poolName, metav1.GetOptions{}) require.NoError(t, err) - insertNewRenderedMachineConfig(ctx, t, mcfgclient, poolName, renderedName, fixtures.OSImageURL) + mc := insertNewRenderedMachineConfig(ctx, t, mcfgclient, poolName, renderedName, fixtures.OSImageURL) mcp.Spec.Configuration.Name = renderedName mcp, err = mcfgclient.MachineconfigurationV1().MachineConfigPools().Update(ctx, mcp, metav1.UpdateOptions{}) require.NoError(t, err) - return mcp + return mcp, mc } -func insertNewRenderedMachineConfig(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, poolName, renderedName string, osImageURL string) { - filename := filepath.Join("/etc", poolName, renderedName) +func insertNewRenderedMachineConfig(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, poolName, renderedName string, osImageURL string) *mcfgv1.MachineConfig { + mc := fixtures.NewObjectsForTest(poolName).RenderedMachineConfig + mc.Name = renderedName + mc.Spec.OSImageURL = osImageURL - file := ctrlcommon.NewIgnFile(filename, renderedName) - mc := testhelpers.NewMachineConfig( - renderedName, - map[string]string{ - ctrlcommon.GeneratedByControllerVersionAnnotationKey: "version-number", - "machineconfiguration.openshift.io/role": poolName, - }, - osImageURL, - []ign3types.File{file}) - _, err := mcfgclient.MachineconfigurationV1().MachineConfigs().Create(ctx, mc, metav1.CreateOptions{}) + apiMC, err := mcfgclient.MachineconfigurationV1().MachineConfigs().Create(ctx, mc, metav1.CreateOptions{}) require.NoError(t, err) + + return apiMC } -func insertNewRenderedMachineConfigWithoutImageChangeAndUpdatePool(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, poolName, renderedName string) *mcfgv1.MachineConfigPool { +func insertNewRenderedMachineConfigWithoutImageChangeAndUpdatePool(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, poolName, renderedName string) (*mcfgv1.MachineConfigPool, *mcfgv1.MachineConfig) { mcp, err := mcfgclient.MachineconfigurationV1().MachineConfigPools().Get(ctx, poolName, metav1.GetOptions{}) require.NoError(t, err) - insertNewRenderedMachineConfigWithoutImageChange(ctx, t, mcfgclient, poolName, renderedName) + mc := insertNewRenderedMachineConfigWithoutImageChange(ctx, t, mcfgclient, poolName, renderedName) mcp.Spec.Configuration.Name = renderedName mcp, err = mcfgclient.MachineconfigurationV1().MachineConfigPools().Update(ctx, mcp, metav1.UpdateOptions{}) require.NoError(t, err) - return mcp + return mcp, mc } -func insertNewRenderedMachineConfigWithoutImageChange(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, poolName, renderedName string) { - filename := filepath.Join("/etc", poolName, renderedName) +func insertNewRenderedMachineConfigWithoutImageChange(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, poolName, renderedName string) *mcfgv1.MachineConfig { + mc := fixtures.NewObjectsForTest(poolName).RenderedMachineConfig + mc.Name = renderedName - file := ctrlcommon.NewIgnFile(filename, renderedName) - mc := testhelpers.NewMachineConfig( - renderedName, - map[string]string{ - ctrlcommon.GeneratedByControllerVersionAnnotationKey: "version-number", - "machineconfiguration.openshift.io/role": poolName, - }, - "", - []ign3types.File{file}) - _, err := mcfgclient.MachineconfigurationV1().MachineConfigs().Create(ctx, mc, metav1.CreateOptions{}) + apiMC, err := mcfgclient.MachineconfigurationV1().MachineConfigs().Create(ctx, mc, metav1.CreateOptions{}) require.NoError(t, err) + + return apiMC } func isMachineOSBuildReachedExpectedCount(ctx context.Context, t *testing.T, mcfgclient mcfgclientset.Interface, mosc *mcfgv1.MachineOSConfig, expected int) { @@ -835,7 +869,7 @@ func TestOSBuildControllerSkipsBuildForLayerOnlyChanges(t *testing.T) { t.Cleanup(cancel) poolName := "worker" - _, mcfgclient, _, _, mosc, firstMosb, mcp, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName) + _, mcfgclient, _, _, mosc, firstMosb, mcp, _, kubeassert := setupOSBuildControllerForTestWithSuccessfulBuild(ctx, t, poolName) isMachineOSBuildReachedExpectedCount(ctx, t, mcfgclient, mosc, 1) assertMachineOSConfigGetsCurrentBuildAnnotation(ctx, t, mcfgclient, mosc, firstMosb) diff --git a/pkg/controller/build/reconciler.go b/pkg/controller/build/reconciler.go index d463562b7b..982217902c 100644 --- a/pkg/controller/build/reconciler.go +++ b/pkg/controller/build/reconciler.go @@ -537,24 +537,23 @@ func (b *buildReconciler) createNewMachineOSBuildOrReuseExisting(ctx context.Con return fmt.Errorf("could not get MachineConfigPool %s for MachineOSConfig %s: %w", mosc.Spec.MachineConfigPool.Name, mosc.Name, err) } + mc, err := b.machineConfigLister.Get(mcp.Spec.Configuration.Name) + if err != nil { + return fmt.Errorf("could not get MachineConfig %s for MachineConfigPool %s: %w", mcp.Spec.Configuration.Name, mcp.Name, err) + } + // Allow builds to retry when pool is degraded only due to BuildDegraded, // but prevent builds for other types of degradation (NodeDegraded, RenderDegraded) if b.shouldPreventBuildDueToDegradation(mcp) { return fmt.Errorf("MachineConfigPool %s is degraded due to non-build issues", mcp.Name) } - // TODO: Consider using a ConfigMap lister to get this value instead of the API server. - osImageURLs, err := ctrlcommon.GetOSImageURLConfig(ctx, b.kubeclient) - if err != nil { - return fmt.Errorf("could not get OSImageURLConfig: %w", err) - } - // Construct a new MachineOSBuild object which has the hashed name attached // to it. mosb, err := buildrequest.NewMachineOSBuild(buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, MachineOSConfig: mosc, MachineConfigPool: mcp, - OSImageURLConfig: osImageURLs, }) if err != nil { @@ -1281,12 +1280,15 @@ func (b *buildReconciler) reconcilePoolChange(ctx context.Context, mcp *mcfgv1.M // This is our trigger point if (oldRendered != newRendered && needsImageRebuild) || firstOptIn == "" { klog.Infof("pool %q: rendered config changed and requires an image rebuild. Verifying if a valid build already exists...", mcp.Name) + mc, err := b.machineConfigLister.Get(mcp.Spec.Configuration.Name) + if err != nil { + return fmt.Errorf("could not get MachineConfig %q for MachineConfigPool %q: %w", mcp.Spec.Configuration.Name, mcp.Name, err) + } - osImageURLs, _ := ctrlcommon.GetOSImageURLConfig(ctx, b.kubeclient) targetMosb, err := buildrequest.NewMachineOSBuild(buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, MachineOSConfig: mosc, MachineConfigPool: mcp, - OSImageURLConfig: osImageURLs, }) if err != nil { return fmt.Errorf("could not generate name for target MOSB: %w", err) @@ -1356,18 +1358,19 @@ func (b *buildReconciler) reuseImageForNewMOSB(ctx context.Context, mosc *mcfgv1 return err } - // Get the osimageurl for our new MOSB object - osImageURLs, err := ctrlcommon.GetOSImageURLConfig(ctx, b.kubeclient) + mc, err := b.machineConfigLister.Get(mcp.Spec.Configuration.Name) if err != nil { return err } + // Build the new MOSB object. this is our "promise", we will eventually check if we will proceed with this newMosb, err := buildrequest.NewMachineOSBuild( buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, MachineOSConfig: mosc, MachineConfigPool: mcp, - OSImageURLConfig: osImageURLs, }) + if err != nil { return err } diff --git a/test/e2e-ocl/onclusterlayering_test.go b/test/e2e-ocl/onclusterlayering_test.go index 781bd80f78..9323265b52 100644 --- a/test/e2e-ocl/onclusterlayering_test.go +++ b/test/e2e-ocl/onclusterlayering_test.go @@ -372,7 +372,14 @@ func TestMachineOSConfigChangeRestartsBuild(t *testing.T) { mcp, err := cs.MachineconfigurationV1Interface.MachineConfigPools().Get(ctx, layeredMCPName, metav1.GetOptions{}) require.NoError(t, err) - firstMosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, cs.GetKubeclient(), mosc, mcp) + mc, err := cs.MachineconfigurationV1Interface.MachineConfigs().Get(ctx, mcp.Spec.Configuration.Name, metav1.GetOptions{}) + require.NoError(t, err) + + firstMosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, + MachineOSConfig: mosc, + MachineConfigPool: mcp, + }) // First, we get a MachineOSBuild started as usual. waitForBuildToStart(t, cs, firstMosb) @@ -382,7 +389,11 @@ func TestMachineOSConfigChangeRestartsBuild(t *testing.T) { apiMosc := helpers.SetContainerfileContentsOnMachineOSConfig(ctx, t, cs.GetMcfgclient(), mosc, "FROM configs AS final\nRUN echo 'hello' > /etc/hello") - moscChangeMosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, cs.GetKubeclient(), apiMosc, mcp) + moscChangeMosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, + MachineOSConfig: apiMosc, + MachineConfigPool: mcp, + }) kubeassert := helpers.AssertClientSet(t, cs).WithContext(ctx) @@ -478,14 +489,21 @@ func TestGracefulBuildFailureRecovery(t *testing.T) { apiMosc.Spec.Containerfile = []mcfgv1.MachineOSContainerfile{} - updated, err := cs.MachineconfigurationV1Interface.MachineOSConfigs().Update(ctx, apiMosc, metav1.UpdateOptions{}) + updatedMosc, err := cs.MachineconfigurationV1Interface.MachineOSConfigs().Update(ctx, apiMosc, metav1.UpdateOptions{}) require.NoError(t, err) mcp, err := cs.MachineconfigurationV1Interface.MachineConfigPools().Get(ctx, layeredMCPName, metav1.GetOptions{}) require.NoError(t, err) + mc, err := cs.MachineconfigurationV1Interface.MachineConfigs().Get(ctx, mcp.Spec.Configuration.Name, metav1.GetOptions{}) + require.NoError(t, err) + // Compute the new MachineOSBuild image name. - moscChangeMosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, cs.GetKubeclient(), updated, mcp) + moscChangeMosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, + MachineOSConfig: updatedMosc, + MachineConfigPool: mcp, + }) // Wait for the second build to start. secondMosb := waitForBuildToStart(t, cs, moscChangeMosb) @@ -1254,7 +1272,14 @@ func TestControllerEventuallyReconciles(t *testing.T) { createMachineOSConfig(t, cs, mosc) - mosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, cs.GetKubeclient(), mosc, mcp) + mc, err := cs.MachineconfigurationV1Interface.MachineConfigs().Get(ctx, mcp.Spec.Configuration.Name, metav1.GetOptions{}) + require.NoError(t, err) + + mosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, + MachineOSConfig: mosc, + MachineConfigPool: mcp, + }) // Wait for the MachineOSBuild to exist. kubeassert := helpers.AssertClientSet(t, cs).WithContext(ctx).Eventually() @@ -1440,7 +1465,7 @@ func TestImageBuildDegradedOnFailureAndClearedOnBuildStart(t *testing.T) { }, } - updated, err := cs.MachineconfigurationV1Interface.MachineOSConfigs().Update(ctx, apiMosc, metav1.UpdateOptions{}) + updatedMosc, err := cs.MachineconfigurationV1Interface.MachineOSConfigs().Update(ctx, apiMosc, metav1.UpdateOptions{}) require.NoError(t, err) t.Logf("Fixed containerfile, waiting for new build to start") @@ -1449,7 +1474,14 @@ func TestImageBuildDegradedOnFailureAndClearedOnBuildStart(t *testing.T) { require.NoError(t, err) // Compute the new MachineOSBuild name - moscChangeMosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, cs.GetKubeclient(), updated, mcp) + mc, err := cs.MachineconfigurationV1Interface.MachineConfigs().Get(ctx, mcp.Spec.Configuration.Name, metav1.GetOptions{}) + require.NoError(t, err) + + moscChangeMosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, + MachineOSConfig: updatedMosc, + MachineConfigPool: mcp, + }) // Wait for the second build to start secondMosb := waitForBuildToStart(t, cs, moscChangeMosb) @@ -1519,7 +1551,7 @@ func TestImageBuildDegradedOnFailureAndClearedOnBuildStart(t *testing.T) { }, } - updated, err = cs.MachineconfigurationV1Interface.MachineOSConfigs().Update(ctx, apiMosc, metav1.UpdateOptions{}) + updatedMosc, err = cs.MachineconfigurationV1Interface.MachineOSConfigs().Update(ctx, apiMosc, metav1.UpdateOptions{}) require.NoError(t, err) t.Logf("Modified containerfile, waiting for third build to start") @@ -1528,8 +1560,16 @@ func TestImageBuildDegradedOnFailureAndClearedOnBuildStart(t *testing.T) { mcp, err = cs.MachineconfigurationV1Interface.MachineConfigPools().Get(ctx, layeredMCPName, metav1.GetOptions{}) require.NoError(t, err) + //Get the updated MC to compute the new build + mc, err = cs.MachineconfigurationV1Interface.MachineConfigs().Get(ctx, mcp.Spec.Configuration.Name, metav1.GetOptions{}) + require.NoError(t, err) + // Compute the new MachineOSBuild name for the third build - thirdMoscMosb := buildrequest.NewMachineOSBuildFromAPIOrDie(ctx, cs.GetKubeclient(), updated, mcp) + thirdMoscMosb := buildrequest.NewMachineOSBuildOrDie(buildrequest.MachineOSBuildOpts{ + MachineConfig: mc, + MachineOSConfig: updatedMosc, + MachineConfigPool: mcp, + }) // Wait for the third build to start thirdMosb := waitForBuildToStart(t, cs, thirdMoscMosb)