Skip to content

Commit

Permalink
MGMT-15150: Use same installer binary for all platform types (#5334)
Browse files Browse the repository at this point in the history 
* MGMT-15150: Use same installer binary for all platform types

There's nothing special about platform:none that requires it to use a
different installer binary. This was originally done (in f9b2f3d) to
avoid a problem with the openshift-baremetal-install binary not being
available for non-x86 targets, but this was resolved by 1d025b8
(MGMT-9206).

* Remove platform argument when extracting installer

Since we no longer use different installer binaries for different
platforms, we no longer need to pass the platform type down. This allows
us to clean up a lot of function signatures.

This partially reverts commit f9b2f3d.
  • Loading branch information
@zaneb
zaneb committed Jul 9, 2023
1 parent 448cb29 commit 820a271
Show file tree
Hide file tree
Showing 10 changed files with 47 additions and 87 deletions.
3 changes: 1 addition & 2 deletions internal/bminventory/inventory.go
View file
Expand Up @@ -6440,15 +6440,14 @@ func (b *bareMetalInventory) GetKnownApprovedHosts(clusterId strfmt.UUID) ([]*co
return b.hostApi.GetKnownApprovedHosts(clusterId)
}

// In case cpu architecture is not x86_64 and platform is baremetal, we should extract openshift-baremetal-installer
// In case cpu architecture is not x86_64, we should extract openshift-baremetal-installer
// from x86_64 release image as there is no x86_64 openshift-baremetal-installer executable in arm image.
// This flow does not affect the multiarch release images and is meant purely for using arm64 release image with the x86 hub.
// Implementation of handling the multiarch images is done directly in the `oc` binary and relies on the fact that `oc adm release extract`
// will automatically use the image matching the Hub's architecture.
func isBaremetalBinaryFromAnotherReleaseImageRequired(cpuArchitecture, version string, platform *models.PlatformType) bool {
return cpuArchitecture != common.MultiCPUArchitecture &&
cpuArchitecture != common.NormalizeCPUArchitecture(runtime.GOARCH) &&
common.PlatformTypeValue(platform) == models.PlatformTypeBaremetal &&
featuresupport.IsFeatureAvailable(models.FeatureSupportLevelIDCLUSTERMANAGEDNETWORKING, version, swag.String(models.ClusterCPUArchitectureArm64))
}

Expand Down
3 changes: 1 addition & 2 deletions internal/ignition/dummy.go
View file
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/openshift/assisted-service/internal/common"
"github.com/openshift/assisted-service/internal/host/hostutil"
"github.com/openshift/assisted-service/models"
"github.com/openshift/assisted-service/pkg/auth"
"github.com/openshift/assisted-service/pkg/s3wrapper"
"github.com/sirupsen/logrus"
Expand All @@ -33,7 +32,7 @@ func NewDummyGenerator(serviceBaseURL string, workDir string, cluster *common.Cl
}

// Generate creates the expected ignition and related files but with nonsense content
func (g *dummyGenerator) Generate(_ context.Context, installConfig []byte, platformType models.PlatformType, authType auth.AuthType) error {
func (g *dummyGenerator) Generate(_ context.Context, installConfig []byte, authType auth.AuthType) error {
toUpload := fileNames[:]
for _, host := range g.cluster.Hosts {
toUpload = append(toUpload, hostutil.IgnitionFileName(host))
Expand Down
6 changes: 3 additions & 3 deletions internal/ignition/ignition.go
View file
Expand Up @@ -202,7 +202,7 @@ var fileNames = [...]string{

// Generator can generate ignition files and upload them to an S3-like service
type Generator interface {
Generate(ctx context.Context, installConfig []byte, platformType models.PlatformType, authType auth.AuthType) error
Generate(ctx context.Context, installConfig []byte, authType auth.AuthType) error
UploadToS3(ctx context.Context) error
UpdateEtcHosts(string) error
}
Expand Down Expand Up @@ -307,7 +307,7 @@ func (g *installerGenerator) UploadToS3(ctx context.Context) error {
}

// Generate generates ignition files and applies modifications.
func (g *installerGenerator) Generate(ctx context.Context, installConfig []byte, platformType models.PlatformType, authType auth.AuthType) error {
func (g *installerGenerator) Generate(ctx context.Context, installConfig []byte, authType auth.AuthType) error {
var err error
log := logutil.FromContext(ctx, g.log)

Expand All @@ -328,7 +328,7 @@ func (g *installerGenerator) Generate(ctx context.Context, installConfig []byte,
MaxTries: oc.DefaultTries, RetryDelay: oc.DefaltRetryDelay}, mirrorRegistriesBuilder)

release, err := g.installerCache.Get(g.installerReleaseImageOverride, g.releaseImageMirror,
g.cluster.PullSecret, ocRelease, platformType)
g.cluster.PullSecret, ocRelease)
if err != nil {
return errors.Wrap(err, "failed to get installer path")
}
Expand Down
8 changes: 4 additions & 4 deletions internal/ignition/mock_ignition.go
View file

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

7 changes: 3 additions & 4 deletions internal/installercache/installercache.go
View file
Expand Up @@ -10,7 +10,6 @@ import (
"time"

"github.com/openshift/assisted-service/internal/oc"
"github.com/openshift/assisted-service/models"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -64,7 +63,7 @@ func New(cacheDir string, storageCapacity int64, log logrus.FieldLogger) *Instal
// Get returns the path to an openshift-baremetal-install binary extracted from
// the referenced release image. Tries the mirror release image first if it's set. It is safe for concurrent use. A cache of
// binaries is maintained to reduce re-downloading of the same release.
func (i *Installers) Get(releaseID, releaseIDMirror, pullSecret string, ocRelease oc.Release, platformType models.PlatformType) (*Release, error) {
func (i *Installers) Get(releaseID, releaseIDMirror, pullSecret string, ocRelease oc.Release) (*Release, error) {
i.Lock()
defer i.Unlock()

Expand All @@ -75,13 +74,13 @@ func (i *Installers) Get(releaseID, releaseIDMirror, pullSecret string, ocReleas
if releaseIDMirror != "" {
releaseImageLocation = releaseIDMirror
}
workdir, binary, path = ocRelease.GetReleaseBinaryPath(releaseImageLocation, i.cacheDir, platformType)
workdir, binary, path = ocRelease.GetReleaseBinaryPath(releaseImageLocation, i.cacheDir)
if _, err = os.Stat(path); os.IsNotExist(err) {
//evict older files if necessary
i.evict()

//extract the binary
_, err = ocRelease.Extract(i.log, releaseID, releaseIDMirror, i.cacheDir, pullSecret, platformType)
_, err = ocRelease.Extract(i.log, releaseID, releaseIDMirror, i.cacheDir, pullSecret)
if err != nil {
return &Release{}, err
}
Expand Down
28 changes: 12 additions & 16 deletions internal/installercache/installercache_test.go
View file
Expand Up @@ -11,7 +11,6 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/openshift/assisted-service/internal/oc"
"github.com/openshift/assisted-service/models"
"github.com/sirupsen/logrus"
)

Expand Down Expand Up @@ -46,16 +45,15 @@ var _ = Describe("installer cache", func() {
fname := filepath.Join(workdir, releaseID)

mockRelease.EXPECT().GetReleaseBinaryPath(
gomock.Any(), gomock.Any(), gomock.Any()).
gomock.Any(), gomock.Any()).
Return(workdir, releaseID, fname)
mockRelease.EXPECT().Extract(gomock.Any(), releaseID,
gomock.Any(), cacheDir, gomock.Any(),
gomock.Any()).
DoAndReturn(func(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string, platformType models.PlatformType) (string, error) {
gomock.Any(), cacheDir, gomock.Any()).
DoAndReturn(func(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string) (string, error) {
err := os.WriteFile(fname, []byte("abcde"), 0600)
return "", err
})
l, err := manager.Get(releaseID, "mirror", "pull-secret", mockRelease, models.PlatformTypeBaremetal)
l, err := manager.Get(releaseID, "mirror", "pull-secret", mockRelease)
Expect(err).ShouldNot(HaveOccurred())

time.Sleep(1 * time.Second)
Expand Down Expand Up @@ -123,16 +121,15 @@ var _ = Describe("installer cache", func() {
fname := filepath.Join(workdir, releaseID)

mockRelease.EXPECT().GetReleaseBinaryPath(
releaseMirrorID, gomock.Any(), gomock.Any()).
releaseMirrorID, gomock.Any()).
Return(workdir, releaseID, fname)
mockRelease.EXPECT().Extract(gomock.Any(), releaseID,
gomock.Any(), cacheDir, gomock.Any(),
gomock.Any()).
DoAndReturn(func(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string, platformType models.PlatformType) (string, error) {
gomock.Any(), cacheDir, gomock.Any()).
DoAndReturn(func(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string) (string, error) {
err := os.WriteFile(fname, []byte("abcde"), 0600)
return "", err
})
_, err := manager.Get(releaseID, releaseMirrorID, "pull-secret", mockRelease, models.PlatformTypeBaremetal)
_, err := manager.Get(releaseID, releaseMirrorID, "pull-secret", mockRelease)
Expect(err).ShouldNot(HaveOccurred())
})

Expand All @@ -143,16 +140,15 @@ var _ = Describe("installer cache", func() {
fname := filepath.Join(workdir, releaseID)

mockRelease.EXPECT().GetReleaseBinaryPath(
releaseID, gomock.Any(), gomock.Any()).
releaseID, gomock.Any()).
Return(workdir, releaseID, fname)
mockRelease.EXPECT().Extract(gomock.Any(), releaseID,
gomock.Any(), cacheDir, gomock.Any(),
gomock.Any()).
DoAndReturn(func(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string, platformType models.PlatformType) (string, error) {
gomock.Any(), cacheDir, gomock.Any()).
DoAndReturn(func(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string) (string, error) {
err := os.WriteFile(fname, []byte("abcde"), 0600)
return "", err
})
_, err := manager.Get(releaseID, releaseMirrorID, "pull-secret", mockRelease, models.PlatformTypeBaremetal)
_, err := manager.Get(releaseID, releaseMirrorID, "pull-secret", mockRelease)
Expect(err).ShouldNot(HaveOccurred())
})
})
Expand Down
17 changes: 8 additions & 9 deletions internal/oc/mock_release.go
View file

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

26 changes: 9 additions & 17 deletions internal/oc/release.go
View file
Expand Up @@ -13,7 +13,6 @@ import (
"github.com/hashicorp/go-version"
operatorv1alpha1 "github.com/openshift/api/operator/v1alpha1"
"github.com/openshift/assisted-service/internal/common"
"github.com/openshift/assisted-service/models"
"github.com/openshift/assisted-service/pkg/executer"
"github.com/openshift/assisted-service/pkg/mirrorregistries"
"github.com/patrickmn/go-cache"
Expand Down Expand Up @@ -47,8 +46,8 @@ type Release interface {
GetOpenshiftVersion(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error)
GetMajorMinorVersion(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error)
GetReleaseArchitecture(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) ([]string, error)
GetReleaseBinaryPath(releaseImage string, cacheDir string, platformType models.PlatformType) (workdir string, binary string, path string)
Extract(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string, platformType models.PlatformType) (string, error)
GetReleaseBinaryPath(releaseImage string, cacheDir string) (workdir string, binary string, path string)
Extract(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string) (string, error)
}

type imageValue struct {
Expand Down Expand Up @@ -314,7 +313,7 @@ func (r *release) getOpenshiftVersionFromRelease(log logrus.FieldLogger, release

// Extract openshift-baremetal-install binary from releaseImageMirror if provided.
// Else extract from the source releaseImage
func (r *release) Extract(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string, platformType models.PlatformType) (string, error) {
func (r *release) Extract(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, cacheDir string, pullSecret string) (string, error) {
var path string
var err error
if releaseImage == "" && releaseImageMirror == "" {
Expand All @@ -329,13 +328,13 @@ func (r *release) Extract(log logrus.FieldLogger, releaseImage string, releaseIm

if releaseImageMirror != "" {
//TODO: Get mirror registry certificate from install-config
path, err = r.extractFromRelease(log, releaseImageMirror, cacheDir, pullSecret, true, platformType, icspFile)
path, err = r.extractFromRelease(log, releaseImageMirror, cacheDir, pullSecret, true, icspFile)
if err != nil {
log.WithError(err).Errorf("failed to extract openshift-baremetal-install from mirror release image %s", releaseImageMirror)
return "", err
}
} else {
path, err = r.extractFromRelease(log, releaseImage, cacheDir, pullSecret, false, platformType, icspFile)
path, err = r.extractFromRelease(log, releaseImage, cacheDir, pullSecret, false, icspFile)
if err != nil {
log.WithError(err).Errorf("failed to extract openshift-baremetal-install from release image %s", releaseImage)
return "", err
Expand All @@ -344,24 +343,17 @@ func (r *release) Extract(log logrus.FieldLogger, releaseImage string, releaseIm
return path, err
}

func (r *release) GetReleaseBinaryPath(releaseImage string, cacheDir string, platformType models.PlatformType) (workdir string, binary string, path string) {
// Using platform type as an indication for which openshift install binary to use
// (e.g. as non-x86_64 clusters should use the openshift-install binary).
if platformType == models.PlatformTypeNone {
binary = "openshift-install"
} else {
binary = "openshift-baremetal-install"
}

func (r *release) GetReleaseBinaryPath(releaseImage string, cacheDir string) (workdir string, binary string, path string) {
workdir = filepath.Join(cacheDir, releaseImage)
binary = "openshift-baremetal-install"
path = filepath.Join(workdir, binary)
return
}

// extractFromRelease returns the path to an openshift-baremetal-install binary extracted from
// the referenced release image.
func (r *release) extractFromRelease(log logrus.FieldLogger, releaseImage, cacheDir, pullSecret string, insecure bool, platformType models.PlatformType, icspFile string) (string, error) {
workdir, binary, path := r.GetReleaseBinaryPath(releaseImage, cacheDir, platformType)
func (r *release) extractFromRelease(log logrus.FieldLogger, releaseImage, cacheDir, pullSecret string, insecure bool, icspFile string) (string, error) {
workdir, binary, path := r.GetReleaseBinaryPath(releaseImage, cacheDir)
log.Infof("extracting %s binary to %s", binary, workdir)
err := os.MkdirAll(workdir, 0755)
if err != nil {
Expand Down

0 comments on commit 820a271

Please sign in to comment.