Review suggestions

Signed-off-by: Pawan Pinjarkar <ppinjark@redhat.com>
2026-02-05 15:47:14 +01:00 · 2023-09-05 16:44:40 -04:00
parent f84c94ead4
commit 021bcfcf7a
12 changed files with 135 additions and 53 deletions
--- a/cmd/openshift-install/testdata/agent/agentconfigtemplate/agent-config-template.txt
+++ b/cmd/openshift-install/testdata/agent/agentconfigtemplate/agent-config-template.txt
@@ -21,7 +21,7 @@ metadata:
  namespace: cluster0
 # All fields are optional
 rendezvousIP: your-node0-ip
-bootArtifactsBaseURL: http://user-specified-pxe-infra.com
+bootArtifactsBaseURL: http://user-specified-infra.com
 additionalNTPSources:
 - 0.rhel.pool.ntp.org
 - 1.rhel.pool.ntp.org
--- a/cmd/openshift-install/testdata/agent/pxe/configurations/sno.txt
+++ b/cmd/openshift-install/testdata/agent/pxe/configurations/sno.txt
@@ -11,8 +11,8 @@ exists $WORK/boot-artifacts/agent.x86_64.ipxe
 exists $WORK/auth/kubeconfig
 exists $WORK/auth/kubeadmin-password

-grep 'initrd --name initrd http://user-specified-pxe-infra.com/agent.x86_64-initrd.img' $WORK/pxe/agent.x86_64.ipxe
-grep 'kernel http://user-specified-pxe-infra.com/agent.x86_64-vmlinuz initrd=initrd coreos.live.rootfs_url=http://user-specified-pxe-infra.com/agent.x86_64-rootfs.img .*ignition.firstboot ignition.platform.id=metal' $WORK/pxe/agent.x86_64.ipxe
+grep 'initrd --name initrd http://user-specified-pxe-infra.com/agent.x86_64-initrd.img' $WORK/boot-artifacts/agent.x86_64.ipxe
+grep 'kernel http://user-specified-pxe-infra.com/agent.x86_64-vmlinuz initrd=initrd coreos.live.rootfs_url=http://user-specified-pxe-infra.com/agent.x86_64-rootfs.img .*ignition.firstboot ignition.platform.id=metal' $WORK/boot-artifacts/agent.x86_64.ipxe
 ! grep 'coreos.liveiso=' $WORK/boot-artifacts/agent.x86_64.ipxe


@@ -48,7 +48,7 @@ metadata:
  name: ostest
  namespace: cluster0
 rendezvousIP: 192.168.111.20
-bootArtifactsBaseUrl: http://user-specified-pxe-infra.com
+bootArtifactsBaseURL: http://user-specified-pxe-infra.com

 -- expected/agent.x86_64.ipxe --
 #!ipxe
--- a/cmd/openshift-install/testdata/agent/pxe/configurations/sno_arm.txt
+++ b/cmd/openshift-install/testdata/agent/pxe/configurations/sno_arm.txt
@@ -11,8 +11,8 @@ exists $WORK/boot-artifacts/agent.aarch64.ipxe
 exists $WORK/auth/kubeconfig
 exists $WORK/auth/kubeadmin-password

-grep 'initrd --name initrd http://user-specified-pxe-infra.com/agent.aarch64-initrd.img' $WORK/pxe/agent.aarch64.ipxe
-grep 'kernel http://user-specified-pxe-infra.com/agent.aarch64-vmlinuz initrd=initrd coreos.live.rootfs_url=http://user-specified-pxe-infra.com/agent.aarch64-rootfs.img .*ignition.firstboot ignition.platform.id=metal' $WORK/pxe/agent.aarch64.ipxe
+grep 'initrd --name initrd http://user-specified-pxe-infra.com/agent.aarch64-initrd.img' $WORK/boot-artifacts/agent.aarch64.ipxe
+grep 'kernel http://user-specified-pxe-infra.com/agent.aarch64-vmlinuz initrd=initrd coreos.live.rootfs_url=http://user-specified-pxe-infra.com/agent.aarch64-rootfs.img .*ignition.firstboot ignition.platform.id=metal' $WORK/boot-artifacts/agent.aarch64.ipxe
 ! grep 'coreos.liveiso=' $WORK/boot-artifacts/agent.aarch64.ipxe

 -- install-config.yaml --
@@ -49,7 +49,7 @@ metadata:
  name: ostest
  namespace: cluster0
 rendezvousIP: 192.168.111.20
-bootArtifactsBaseUrl: http://user-specified-pxe-infra.com
+bootArtifactsBaseURL: http://user-specified-pxe-infra.com

 -- expected/agent.aarch64.ipxe --
 #!ipxe
--- a/cmd/openshift-install/testdata/agent/pxe/validations/sno_invalid_bootArtifactsBaseUrl.txt
+++ b/cmd/openshift-install/testdata/agent/pxe/validations/sno_invalid_bootArtifactsBaseUrl.txt
@@ -41,4 +41,4 @@ metadata:
  name: ostest
  namespace: cluster0
 rendezvousIP: 192.168.111.20
-bootArtifactsBaseUrl: not-a-url
+bootArtifactsBaseURL: not-a-url
--- a/docs/user/agent/external-platform.md
+++ b/docs/user/agent/external-platform.md
@@ -0,0 +1,62 @@
+# External platform configurations
+
+This document provides comprehensive details for configuring the external platform using the agent-based installer for OpenShift. It covers the minimum version requirement, 
+configuration options, and the generation of minimal ISOs for the specified platform.
+
+
+## Minimum OpenShift Version
+The agent-based installer for OpenShift requires a minimum OpenShift version of 4.14 to support external platforms.
+
+## Configuring the External Platform
+The external platform can be specified using either the install-config.yaml or by utilizing ZTP (Zero Touch Provisioning) manifests in agent-cluster-install.yaml. 
+When configuring the external platform, ensure that the platformName field is set to `oci`.
+
+__install-config.yaml__
+```
+apiVersion: v1
+baseDomain: test.metalkube.org
+metadata:
+  name: ostest
+  namespace: cluster0
+................
+................
+................
+................
+platform:
+    external:
+      platformName: oci
+```
+
+__agent-cluster-install.yaml__
+```
+kind: AgentClusterInstall
+metadata:
+  name: ostest
+  namespace: cluster0
+spec:
+  external:
+    platformName: oci
+................
+................
+................
+................
+```
+
+## Generation of Minimal ISOs
+For the external platform, the agent-based installer always generates a minimal ISO, but may or may not generate the rootfs file explicitly. A minimal ISO is similar to the full ISO generated for other platforms, with the distinction that it does not contain the rootfs file within it.
+
+When generating the minimal ISO, the agent-based installer follows these steps:
+
+1. Deletes the rootfs image file from the RHCOS (Red Hat CoreOS) base ISO.
+2. Updates the grub configuration parameter `coreos.live.rootfs_url=` with the URL for the rootfs image file location.
+3. Users can specify the rootfs URL via an optional field named `bootArtifactsBaseURL` in the `agent-config.yaml`.
+
+# Downloading Rootfs Image
+When agent nodes are booted with the minimal ISO, the actual rootfs image file is dynamically downloaded into memory from the URL provided internally by the agent-based installer in the grub configuration.
+
+## RootFS URL Configuration
+When running `openshift-install agent create image`
+
+- If the rootFS URL is specified via the `bootArtifactsBaseURL` field in `agent-config.yaml`, the agent-based installer embeds the specified URL into the grub configuration. It also generates a minimal ISO along with the rootfs.img file in the `boot-artifacts` directory. For IPV6 disconnected cluster installations, ensure that the agent-installer generated rootfs image file is uploaded to the URL specified in `bootArtifactsBaseURL` before booting the nodes with the minimal ISO.
+
+- If the rootFS URL is not specified via `bootArtifactsBaseURL` in `agent-config.yaml`, the agent-based installer embeds the default rootfs URL from the RHCOS streams file into the grub configuration. In this case, only a minimal ISO is generated. This is particularly useful for IPV4 connected cluster installations, as the default rootfs URL from the RHCOS streams is readily accessible in connected environments.
--- a/pkg/asset/agent/agentconfig/agent_config.go
+++ b/pkg/asset/agent/agentconfig/agent_config.go
@@ -23,9 +23,9 @@ var (

 // AgentConfig reads the agent-config.yaml file.
 type AgentConfig struct {
-	File                 *asset.File
-	Config               *agent.Config
-	Template             string
+	File     *asset.File
+	Config   *agent.Config
+	Template string
 }

 var _ asset.WritableAsset = (*AgentConfig)(nil)
@@ -58,7 +58,7 @@ metadata:
  namespace: cluster0
 # All fields are optional
 rendezvousIP: your-node0-ip
-bootArtifactsBaseURL: http://user-specified-pxe-infra.com
+bootArtifactsBaseURL: http://user-specified-infra.com
 additionalNTPSources:
 - 0.rhel.pool.ntp.org
 - 1.rhel.pool.ntp.org
--- a/pkg/asset/agent/agentconfig/agent_config_test.go
+++ b/pkg/asset/agent/agentconfig/agent_config_test.go
@@ -352,7 +352,7 @@ rendezvousIP: 192.168.111.80`,
 apiVersion: v1alpha1
 metadata:
  name: agent-config-cluster0
-  bootArtifactsBaseURL: not-a-valid-url`,
+bootArtifactsBaseURL: not-a-valid-url`,

 			expectedFound: false,
 			expectedError: "invalid Agent Config configuration: bootArtifactsBaseURL: Invalid value: \"not-a-valid-url\": invalid URI \"not-a-valid-url\" (no scheme)",
--- a/pkg/asset/agent/image/agentartifacts.go
+++ b/pkg/asset/agent/image/agentartifacts.go
@@ -16,7 +16,7 @@ import (

 const (
 	// bootArtifactsPath is the path where boot files are created.
-	// e.g. initrd, kernel and rootfs
+	// e.g. initrd, kernel and rootfs.
 	bootArtifactsPath = "boot-artifacts"
 )

@@ -30,7 +30,7 @@ type AgentArtifacts struct {
 	IgnitionByte         []byte
 	Kargs                []byte
 	ISOPath              string
-	BootArtifactsBaseUrl string
+	BootArtifactsBaseURL string
 }

 // Dependencies returns the assets on which the AgentArtifacts asset depends.
@@ -65,7 +65,9 @@ func (a *AgentArtifacts) Generate(dependencies asset.Parents) error {
 	a.IgnitionByte = ignitionByte
 	a.ISOPath = baseIso.File.Filename
 	a.Kargs = kargs.KernelCmdLine()
-	a.BootArtifactsBaseUrl = strings.Trim(agentconfig.Config.BootArtifactsBaseURL, "/")
+	if agentconfig.Config != nil {
+		a.BootArtifactsBaseURL = strings.Trim(agentconfig.Config.BootArtifactsBaseURL, "/")
+	}

 	agentTuiFiles, err := a.fetchAgentTuiFiles(agentManifests.ClusterImageSet.Spec.ReleaseImage, agentManifests.GetPullSecretData(), registriesConf.MirrorConfig)
 	if err != nil {
@@ -189,14 +191,17 @@ func (a *AgentArtifacts) Files() []*asset.File {
 	return []*asset.File{}
 }

-func extractRootFS(bootArtifactsFullPath, agentISOPath, arch string) error {
+func createDir(bootArtifactsFullPath string) error {
 	os.RemoveAll(bootArtifactsFullPath)

 	err := os.Mkdir(bootArtifactsFullPath, 0750)
 	if err != nil {
 		return err
 	}
+	return nil
+}

+func extractRootFS(bootArtifactsFullPath, agentISOPath, arch string) error {
 	agentRootfsimgFile := filepath.Join(bootArtifactsFullPath, fmt.Sprintf("agent.%s-rootfs.img", arch))
 	rootfsReader, err := os.Open(filepath.Join(agentISOPath, "images", "pxeboot", "rootfs.img"))
 	if err != nil {
@@ -204,7 +209,7 @@ func extractRootFS(bootArtifactsFullPath, agentISOPath, arch string) error {
 	}
 	defer rootfsReader.Close()

-	err = copy(agentRootfsimgFile, rootfsReader)
+	err = copyfile(agentRootfsimgFile, rootfsReader)
 	if err != nil {
 		return err
 	}
--- a/pkg/asset/agent/image/agentimage.go
+++ b/pkg/asset/agent/image/agentimage.go
@@ -8,11 +8,12 @@ import (
 	"path/filepath"
 	"regexp"

+	"github.com/sirupsen/logrus"
+
 	"github.com/openshift/assisted-image-service/pkg/isoeditor"
+	hiveext "github.com/openshift/assisted-service/api/hiveextension/v1beta1"
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/asset/agent/manifests"
-	"github.com/openshift/installer/pkg/types/external"
-	"github.com/sirupsen/logrus"
 )

 const (
@@ -28,7 +29,7 @@ type AgentImage struct {
 	isoPath              string
 	rootFSURL            string
 	bootArtifactsBaseURL string
-	platform             string
+	platform             hiveext.PlatformType
 }

 var _ asset.WritableAsset = (*AgentImage)(nil)
@@ -53,7 +54,7 @@ func (a *AgentImage) Generate(dependencies asset.Parents) error {
 	a.rendezvousIP = agentArtifacts.RendezvousIP
 	a.tmpPath = agentArtifacts.TmpPath
 	a.isoPath = agentArtifacts.ISOPath
-	a.bootArtifactsBaseURL = agentArtifacts.BootArtifactsBaseUrl
+	a.bootArtifactsBaseURL = agentArtifacts.BootArtifactsBaseURL

 	volumeID, err := isoeditor.VolumeIdentifier(a.isoPath)
 	if err != nil {
@@ -61,24 +62,20 @@ func (a *AgentImage) Generate(dependencies asset.Parents) error {
 	}
 	a.volumeID = volumeID

-	// a.platform = strings.ToLower(string(agentManifests.AgentClusterInstall.Spec.PlatformType))
-	// temp change
-	a.platform = "external"
-	if a.platform == external.Name {
-		defaultRootFSURL, err := baseIso.getRootFSURL(a.cpuArch)
-		if err != nil {
-			return err
-		}
-
+	a.platform = agentManifests.AgentClusterInstall.Spec.PlatformType
+	if a.platform == hiveext.ExternalPlatformType {
 		// when the bootArtifactsBaseURL is specified, construct the custom rootfs URL
 		if a.bootArtifactsBaseURL != "" {
 			a.rootFSURL = fmt.Sprintf("%s/%s", a.bootArtifactsBaseURL, fmt.Sprintf("agent.%s-rootfs.img", a.cpuArch))
-			logrus.Debugf("Using custom rootfs URL")
-
+			logrus.Debugf("Using custom rootfs URL: %s", a.rootFSURL)
 		} else {
-			// we'll default to the URL from the RHCOS streams file
+			// Default to the URL from the RHCOS streams file
+			defaultRootFSURL, err := baseIso.getRootFSURL(a.cpuArch)
+			if err != nil {
+				return err
+			}
 			a.rootFSURL = defaultRootFSURL
-			logrus.Debugf("Using default rootfs URL")
+			logrus.Debugf("Using default rootfs URL: %s", a.rootFSURL)
 		}
 	}

@@ -193,32 +190,43 @@ func (a *AgentImage) PersistToFile(directory string) error {
 	os.Remove(agentIsoFile)

 	var err error
-	// For external platform when the bootArtifactsBaseUrl is specified,
+	// For external platform when the bootArtifactsBaseURL is specified,
 	// output the rootfs file alongside the minimal ISO
-	if a.platform == external.Name && a.bootArtifactsBaseURL != "" {
-		bootArtifactsFullPath := filepath.Join(directory, bootArtifactsPath)
-		err := extractRootFS(bootArtifactsFullPath, a.tmpPath, a.cpuArch)
-		if err != nil {
-			return err
+	if a.platform == hiveext.ExternalPlatformType {
+		if a.bootArtifactsBaseURL != "" {
+			bootArtifactsFullPath := filepath.Join(directory, bootArtifactsPath)
+			err := createDir(bootArtifactsFullPath)
+			if err != nil {
+				return err
+			}
+			err = extractRootFS(bootArtifactsFullPath, a.tmpPath, a.cpuArch)
+			if err != nil {
+				return err
+			}
+			logrus.Infof("RootFS file created in: %s. Upload it at %s", bootArtifactsFullPath, a.rootFSURL)
 		}
-		logrus.Infof("RootFS file created in: %s. Upload it at %s", bootArtifactsFullPath, a.rootFSURL)
 		err = isoeditor.CreateMinimalISO(a.tmpPath, a.volumeID, a.rootFSURL, a.cpuArch, agentIsoFile)
 		if err != nil {
 			return err
 		}
+		logrus.Infof("Generated minimal ISO at %s", agentIsoFile)
 	} else {
 		// Generate full ISO
 		err = isoeditor.Create(agentIsoFile, a.tmpPath, a.volumeID)
 		if err != nil {
 			return err
 		}
+		logrus.Infof("Generated ISO at %s", agentIsoFile)
 	}
-	logrus.Infof("Generated ISO at %s", agentIsoFile)

 	err = os.WriteFile(filepath.Join(directory, "rendezvousIP"), []byte(a.rendezvousIP), 0o644) //nolint:gosec // no sensitive info
 	if err != nil {
 		return err
 	}
+	// For external platform OCI, add CCM manifests in the openshift directory.
+	if a.platform == hiveext.ExternalPlatformType {
+		logrus.Infof("When using %s oci platform, always make sure CCM manifests were added in the %s directory.", hiveext.ExternalPlatformType, manifests.OpenshiftManifestDir())
+	}

 	return nil
 }
--- a/pkg/asset/agent/image/agentpxefiles.go
+++ b/pkg/asset/agent/image/agentpxefiles.go
@@ -9,14 +9,12 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"strings"

 	"github.com/coreos/stream-metadata-go/arch"
 	"github.com/sirupsen/logrus"

 	"github.com/openshift/assisted-image-service/pkg/isoeditor"
 	"github.com/openshift/installer/pkg/asset"
-
 	"github.com/openshift/installer/pkg/types"
 )

@@ -58,7 +56,7 @@ func (a *AgentPXEFiles) Generate(dependencies asset.Parents) error {

 	a.imageReader = custom
 	a.cpuArch = agentArtifacts.CPUArch
-	a.bootArtifactsBaseURL = agentArtifacts.BootArtifactsBaseUrl
+	a.bootArtifactsBaseURL = agentArtifacts.BootArtifactsBaseURL

 	kernelArgs, err := getKernelArgs(filepath.Join(a.tmpPath, "coreos", "kargs.json"))
 	if err != nil {
@@ -79,13 +77,18 @@ func (a *AgentPXEFiles) PersistToFile(directory string) error {
 	defer a.imageReader.Close()
 	bootArtifactsFullPath := filepath.Join(directory, bootArtifactsPath)

-	err := extractRootFS(bootArtifactsFullPath, a.tmpPath, a.cpuArch)
+	err := createDir(bootArtifactsFullPath)
+	if err != nil {
+		return err
+	}
+
+	err = extractRootFS(bootArtifactsFullPath, a.tmpPath, a.cpuArch)
 	if err != nil {
 		return err
 	}

 	agentInitrdFile := filepath.Join(bootArtifactsFullPath, fmt.Sprintf("agent.%s-initrd.img", a.cpuArch))
-	err = copy(agentInitrdFile, a.imageReader)
+	err = copyfile(agentInitrdFile, a.imageReader)
 	if err != nil {
 		return err
 	}
@@ -103,12 +106,12 @@ func (a *AgentPXEFiles) PersistToFile(directory string) error {
 			panic(err)
 		}
 		defer gzipReader.Close()
-		err = copy(agentVmlinuzFile, gzipReader)
+		err = copyfile(agentVmlinuzFile, gzipReader)
 		if err != nil {
 			return err
 		}
 	} else {
-		err = copy(agentVmlinuzFile, kernelReader)
+		err = copyfile(agentVmlinuzFile, kernelReader)
 		if err != nil {
 			return err
 		}
@@ -145,7 +148,7 @@ func (a *AgentPXEFiles) Files() []*asset.File {
 	return []*asset.File{}
 }

-func copy(filepath string, src io.Reader) error {
+func copyfile(filepath string, src io.Reader) error {
 	output, err := os.Create(filepath)
 	if err != nil {
 		return err
--- a/pkg/asset/agent/image/baseiso.go
+++ b/pkg/asset/agent/image/baseiso.go
@@ -74,7 +74,7 @@ func downloadIso(archName string) (string, error) {
 	return "", fmt.Errorf("no ISO found to download for %s", archName)
 }

-// Fetch RootFS URL using the rhcos.json
+// Fetch RootFS URL using the rhcos.json.
 func (i *BaseIso) getRootFSURL(archName string) (string, error) {
 	streamArch, err := getStreamArch(archName)
 	if err != nil {
@@ -84,7 +84,6 @@ func (i *BaseIso) getRootFSURL(archName string) (string, error) {
 		if format, ok := artifacts.Formats["pxe"]; ok {
 			rootFSUrl := format.Rootfs.Location
 			return rootFSUrl, nil
-
 		}
 	} else {
 		return "", errors.Wrap(err, "invalid artifact")
--- a/pkg/asset/agent/manifests/extramanifests.go
+++ b/pkg/asset/agent/manifests/extramanifests.go
@@ -59,3 +59,8 @@ func (em *ExtraManifests) Load(f asset.FileFetcher) (found bool, err error) {

 	return len(em.FileList) > 0, nil
 }
+
+// OpenshiftManifestDir returns the name of directory to add extra manifests.
+func OpenshiftManifestDir() string {
+	return openshiftManifestDir
+}