Skip to content

Commit 96a5f14

Browse files
committed
cleanup implementation
Signed-off-by: Urvashi <[email protected]>
1 parent cf744c5 commit 96a5f14

File tree

7 files changed

+177
-310
lines changed

7 files changed

+177
-310
lines changed

pkg/controller/bootstrap/bootstrap.go

Lines changed: 19 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ import (
1818
kscheme "k8s.io/client-go/kubernetes/scheme"
1919
"k8s.io/klog/v2"
2020

21+
"github.com/containers/image/v5/docker/reference"
22+
"github.com/opencontainers/go-digest"
2123
apicfgv1 "github.com/openshift/api/config/v1"
2224
apicfgv1alpha1 "github.com/openshift/api/config/v1alpha1"
2325
mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
2426
apioperatorsv1alpha1 "github.com/openshift/api/operator/v1alpha1"
2527
buildconstants "github.com/openshift/machine-config-operator/pkg/controller/build/constants"
26-
"github.com/containers/image/v5/docker/reference"
27-
"github.com/opencontainers/go-digest"
2828
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
2929
containerruntimeconfig "github.com/openshift/machine-config-operator/pkg/controller/container-runtime-config"
3030
kubeletconfig "github.com/openshift/machine-config-operator/pkg/controller/kubelet-config"
@@ -96,7 +96,6 @@ func (b *Bootstrap) Run(destDir string) error {
9696
imagePolicies []*apicfgv1.ImagePolicy
9797
imgCfg *apicfgv1.Image
9898
apiServer *apicfgv1.APIServer
99-
secrets []*corev1.Secret
10099
)
101100
for _, info := range infos {
102101
if info.IsDir() {
@@ -162,8 +161,6 @@ func (b *Bootstrap) Run(destDir string) error {
162161
if obj.GetName() == ctrlcommon.APIServerInstanceName {
163162
apiServer = obj
164163
}
165-
case *corev1.Secret:
166-
secrets = append(secrets, obj)
167164
default:
168165
klog.Infof("skipping %q [%d] manifest because of unhandled %T", file.Name(), idx+1, obji)
169166
}
@@ -251,10 +248,8 @@ func (b *Bootstrap) Run(destDir string) error {
251248
if err != nil {
252249
return fmt.Errorf("failed to create pre-built image MachineConfigs: %w", err)
253250
}
254-
if len(preBuiltImageMCs) > 0 {
255-
configs = append(configs, preBuiltImageMCs...)
256-
klog.Infof("Successfully created %d pre-built image component MachineConfigs for hybrid OCL.", len(preBuiltImageMCs))
257-
}
251+
configs = append(configs, preBuiltImageMCs...)
252+
klog.Infof("Successfully created %d pre-built image component MachineConfigs for hybrid OCL.", len(preBuiltImageMCs))
258253
}
259254

260255
fpools, gconfigs, err := render.RunBootstrap(pools, configs, cconfig)
@@ -303,54 +298,6 @@ func (b *Bootstrap) Run(destDir string) error {
303298
}
304299
}
305300

306-
// Write MachineOSConfigs to machine-os-configs directory
307-
// These will be created by the MCO controller after cluster startup
308-
if len(machineOSConfigs) > 0 {
309-
mosconfigdir := filepath.Join(destDir, "machine-os-configs")
310-
if err := os.MkdirAll(mosconfigdir, 0o764); err != nil {
311-
return err
312-
}
313-
for _, mosc := range machineOSConfigs {
314-
buf := bytes.Buffer{}
315-
err := encoder.Encode(mosc, &buf)
316-
if err != nil {
317-
return err
318-
}
319-
path := filepath.Join(mosconfigdir, fmt.Sprintf("%s.yaml", mosc.Name))
320-
// Disable gosec here to avoid throwing
321-
// G306: Expect WriteFile permissions to be 0600 or less
322-
// #nosec
323-
if err := os.WriteFile(path, buf.Bytes(), 0o664); err != nil {
324-
return err
325-
}
326-
}
327-
klog.Infof("Successfully wrote %d MachineOSConfig manifests for post-bootstrap creation", len(machineOSConfigs))
328-
}
329-
330-
// Write Secrets to secrets directory
331-
// These will be created by the MCO controller after cluster startup
332-
if len(secrets) > 0 {
333-
secretsdir := filepath.Join(destDir, "secrets")
334-
if err := os.MkdirAll(secretsdir, 0o764); err != nil {
335-
return err
336-
}
337-
secretEncoder := codecFactory.EncoderForVersion(serializer, corev1.SchemeGroupVersion)
338-
for _, secret := range secrets {
339-
buf := bytes.Buffer{}
340-
err := secretEncoder.Encode(secret, &buf)
341-
if err != nil {
342-
return err
343-
}
344-
path := filepath.Join(secretsdir, fmt.Sprintf("%s.yaml", secret.Name))
345-
// Disable gosec here to avoid throwing
346-
// G306: Expect WriteFile permissions to be 0600 or less
347-
// #nosec
348-
if err := os.WriteFile(path, buf.Bytes(), 0o664); err != nil {
349-
return err
350-
}
351-
}
352-
klog.Infof("Successfully wrote %d Secret manifests for post-bootstrap creation", len(secrets))
353-
}
354301

355302
// If an apiServer object exists, write it to /etc/mcs/bootstrap/api-server/api-server.yaml
356303
// so that bootstrap MCS can consume it
@@ -452,54 +399,33 @@ func parseManifests(filename string, r io.Reader) ([]manifest, error) {
452399
// createPreBuiltImageMachineConfigs creates component MachineConfigs that set osImageURL for pools
453400
// that have associated MachineOSConfigs with pre-built image annotations.
454401
// These component MCs will be automatically merged into rendered MCs by the render controller.
402+
// This function performs strict validation at bootstrap time and will fail if:
403+
// - A MachineOSConfig is missing the pre-built image annotation
404+
// - The pre-built image format or digest is invalid
455405
func createPreBuiltImageMachineConfigs(machineOSConfigs []*mcfgv1.MachineOSConfig, pools []*mcfgv1.MachineConfigPool) ([]*mcfgv1.MachineConfig, error) {
456406
var preBuiltImageMCs []*mcfgv1.MachineConfig
457407

458-
// Create a map of pool names to pre-built images
459-
poolToPreBuiltImage := make(map[string]string)
460-
408+
// At bootstrap time, we require ALL MachineOSConfigs to have pre-built images
409+
// This is a strict requirement for day-0 hybrid OCL support
461410
for _, mosc := range machineOSConfigs {
462-
// Check if this MachineOSConfig has a pre-built image annotation
463411
preBuiltImage, hasPreBuiltImage := mosc.Annotations[buildconstants.PreBuiltImageAnnotationKey]
464412
if !hasPreBuiltImage || preBuiltImage == "" {
465-
continue
413+
return nil, fmt.Errorf("MachineOSConfig %s is missing required annotation %s for bootstrap pre-built image support",
414+
mosc.Name, buildconstants.PreBuiltImageAnnotationKey)
466415
}
467416

468-
// Validate the pre-built image before proceeding
469-
if err := validatePreBuiltImage(preBuiltImage); err != nil {
470-
return nil, fmt.Errorf("invalid pre-built image %q for MachineOSConfig %s: %w", preBuiltImage, mosc.Name, err)
471-
}
417+
poolName := mosc.Spec.MachineConfigPool.Name
472418

473-
klog.Infof("Found MachineOSConfig %s with pre-built image: %s for pool %s", mosc.Name, preBuiltImage, mosc.Spec.MachineConfigPool.Name)
474-
poolToPreBuiltImage[mosc.Spec.MachineConfigPool.Name] = preBuiltImage
475-
}
476-
477-
// Create component MachineConfigs for each pool with a pre-built image
478-
// The render controller will automatically merge these into the rendered MC based on the role label
479-
for poolName, preBuiltImage := range poolToPreBuiltImage {
480-
mcName := fmt.Sprintf("10-prebuiltimage-osimageurl-%s", poolName)
481-
482-
mc := &mcfgv1.MachineConfig{
483-
TypeMeta: metav1.TypeMeta{
484-
APIVersion: mcfgv1.SchemeGroupVersion.String(),
485-
Kind: "MachineConfig",
486-
},
487-
ObjectMeta: metav1.ObjectMeta{
488-
Name: mcName,
489-
Labels: map[string]string{
490-
mcfgv1.MachineConfigRoleLabelKey: poolName,
491-
},
492-
Annotations: map[string]string{
493-
buildconstants.PreBuiltImageAnnotationKey: preBuiltImage,
494-
},
495-
},
496-
Spec: mcfgv1.MachineConfigSpec{
497-
OSImageURL: preBuiltImage,
498-
},
419+
// Validate the pre-built image format and digest
420+
if err := validatePreBuiltImage(preBuiltImage); err != nil {
421+
return nil, fmt.Errorf("invalid pre-built image %q for MachineOSConfig %s (pool %s): %w",
422+
preBuiltImage, mosc.Name, poolName, err)
499423
}
500424

425+
// Create the component MachineConfig
426+
mc := ctrlcommon.CreatePreBuiltImageMachineConfig(poolName, preBuiltImage, buildconstants.PreBuiltImageAnnotationKey)
501427
preBuiltImageMCs = append(preBuiltImageMCs, mc)
502-
klog.Infof("Created component MachineConfig %s with OSImageURL: %s for pool %s", mcName, preBuiltImage, poolName)
428+
klog.Infof("✓ Validated and created component MachineConfig %s with OSImageURL: %s for pool %s", mc.Name, preBuiltImage, poolName)
503429
}
504430

505431
return preBuiltImageMCs, nil
@@ -535,4 +461,3 @@ func validatePreBuiltImage(imageSpec string) error {
535461

536462
return nil
537463
}
538-

pkg/controller/build/constants/constants.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,12 @@ const (
5050
PreBuiltImageSeededAnnotationKey = "machineconfiguration.openshift.io/pre-built-image-seeded"
5151
)
5252

53+
// Component MachineConfig naming for pre-built images
54+
const (
55+
// PreBuiltImageMachineConfigPrefix is the prefix for component MCs that set osImageURL from pre-built images
56+
PreBuiltImageMachineConfigPrefix = "10-prebuiltimage-osimageurl-"
57+
)
58+
5359
// Entitled build secret names
5460
const (
5561
// Name of the etc-pki-entitlement secret from the openshift-config-managed namespace.

pkg/controller/build/helpers.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,42 @@ func hasRebuildAnnotation(mosc *mcfgv1.MachineOSConfig) bool {
256256
return metav1.HasAnnotation(mosc.ObjectMeta, constants.RebuildMachineOSConfigAnnotationKey)
257257
}
258258

259+
// hasPreBuiltImageAnnotation checks if a MachineOSConfig has the pre-built image annotation.
260+
func hasPreBuiltImageAnnotation(mosc *mcfgv1.MachineOSConfig) bool {
261+
_, exists := mosc.Annotations[constants.PreBuiltImageAnnotationKey]
262+
return exists
263+
}
264+
265+
// hasPreBuiltImageSeededAnnotation checks if a MachineOSConfig has been seeded with a pre-built image.
266+
func hasPreBuiltImageSeededAnnotation(mosc *mcfgv1.MachineOSConfig) bool {
267+
_, exists := mosc.Annotations[constants.PreBuiltImageSeededAnnotationKey]
268+
return exists
269+
}
270+
271+
// getPreBuiltImage returns the pre-built image from a MachineOSConfig's annotations.
272+
// Returns the image string and a boolean indicating if it exists and is non-empty.
273+
func getPreBuiltImage(mosc *mcfgv1.MachineOSConfig) (string, bool) {
274+
image, exists := mosc.Annotations[constants.PreBuiltImageAnnotationKey]
275+
return image, exists && image != ""
276+
}
277+
278+
// shouldSeedWithPreBuiltImage determines if a MachineOSConfig should be seeded with a pre-built image.
279+
// Returns true if:
280+
// - The MOSC has a pre-built image annotation
281+
// - The MOSC has NOT been seeded yet
282+
// - The MOSC does NOT have a current build annotation
283+
func shouldSeedWithPreBuiltImage(mosc *mcfgv1.MachineOSConfig) bool {
284+
return hasPreBuiltImageAnnotation(mosc) &&
285+
!hasPreBuiltImageSeededAnnotation(mosc) &&
286+
!hasCurrentBuildAnnotation(mosc)
287+
}
288+
289+
// isPreBuiltImageAwaitingSeeding checks if a MOSC has pre-built image annotation but hasn't been seeded.
290+
// This is useful for skipping normal build workflows when the seeding workflow should handle it.
291+
func isPreBuiltImageAwaitingSeeding(mosc *mcfgv1.MachineOSConfig) bool {
292+
return hasPreBuiltImageAnnotation(mosc) && !hasPreBuiltImageSeededAnnotation(mosc)
293+
}
294+
259295
// Looks at the error chain for the given error and determines if the error
260296
// should be ignored or not based upon whether it is a not found error. If it
261297
// should be ignored, this will log the error as well as the name and kind of

0 commit comments

Comments
 (0)