Skip to content

Commit

Permalink
*: add support for multi-arch rhcos image lookup
Browse files Browse the repository at this point in the history
This adds an architecture parameter to the RHCOS image lookup process
and a corresponding field to MachinePool. This is a backward-compatible
change, defaulting the architecture to AMD64 if none has been specified.
This also enforces that the control plane and compute nodes share an
architecture, since we don't support heterogeneous clusters today.
  • Loading branch information
crawford committed Jan 15, 2020
1 parent b1726d2 commit 2583ba4
Show file tree
Hide file tree
Showing 18 changed files with 97 additions and 27 deletions.
2 changes: 2 additions & 0 deletions docs/user/customization.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ For example, 10.0.0.0/16 represents IP addresses 10.0.0.0 through 10.0.255.255.

The following machine-pool properties are available:

* `architecture` (optional string): Determines the instruction set architecture of the machines in the pool. Currently, heteregeneous clusters are not supported, so all pools must specify the same architecture.
Valid values are `amd64` (the default).
* `hyperthreading` (optional string): Determines the mode of hyperthreading that machines in the pool will utilize.
Valid values are `Enabled` (the default) and `Disabled`.
* `name` (required string): The name of the machine pool.
Expand Down
6 changes: 6 additions & 0 deletions pkg/asset/installconfig/installconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,14 @@ func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) {
Name: "master",
Replicas: pointer.Int64Ptr(3),
Hyperthreading: types.HyperthreadingEnabled,
Architecture: types.ArchitectureAMD64,
},
Compute: []types.MachinePool{
{
Name: "worker",
Replicas: pointer.Int64Ptr(3),
Hyperthreading: types.HyperthreadingEnabled,
Architecture: types.ArchitectureAMD64,
},
},
Platform: types.Platform{
Expand Down Expand Up @@ -145,12 +147,14 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
Name: "master",
Replicas: pointer.Int64Ptr(3),
Hyperthreading: types.HyperthreadingEnabled,
Architecture: types.ArchitectureAMD64,
},
Compute: []types.MachinePool{
{
Name: "worker",
Replicas: pointer.Int64Ptr(3),
Hyperthreading: types.HyperthreadingEnabled,
Architecture: types.ArchitectureAMD64,
},
},
Platform: types.Platform{
Expand Down Expand Up @@ -229,12 +233,14 @@ network:
Name: "master",
Replicas: pointer.Int64Ptr(3),
Hyperthreading: types.HyperthreadingEnabled,
Architecture: types.ArchitectureAMD64,
},
Compute: []types.MachinePool{
{
Name: "worker",
Replicas: pointer.Int64Ptr(3),
Hyperthreading: types.HyperthreadingEnabled,
Architecture: types.ArchitectureAMD64,
},
},
Platform: types.Platform{
Expand Down
14 changes: 9 additions & 5 deletions pkg/asset/manifests/operators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,15 @@ func TestRedactedInstallConfig(t *testing.T) {
ServiceNetwork: []ipnet.IPNet{*ipnet.MustParseCIDR("1.2.3.4/5")},
},
ControlPlane: &types.MachinePool{
Name: "control-plane",
Replicas: pointer.Int64Ptr(3),
Name: "control-plane",
Replicas: pointer.Int64Ptr(3),
Architecture: types.ArchitectureAMD64,
},
Compute: []types.MachinePool{
{
Name: "compute",
Replicas: pointer.Int64Ptr(3),
Name: "compute",
Replicas: pointer.Int64Ptr(3),
Architecture: types.ArchitectureAMD64,
},
},
Platform: types.Platform{
Expand All @@ -59,10 +61,12 @@ func TestRedactedInstallConfig(t *testing.T) {
expectedConfig := createInstallConfig()
expectedYaml := `baseDomain: test-domain
compute:
- name: compute
- architecture: amd64
name: compute
platform: {}
replicas: 3
controlPlane:
architecture: amd64
name: control-plane
platform: {}
replicas: 3
Expand Down
2 changes: 1 addition & 1 deletion pkg/asset/rhcos/bootstrap_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (i *BootstrapImage) Generate(p asset.Parents) error {

// Baremetal IPI launches a local VM for the bootstrap node
// Hence requires the QEMU image to use the libvirt backend
osimage, err = rhcos.QEMU(ctx)
osimage, err = rhcos.QEMU(ctx, config.ControlPlane.Architecture)
default:
// other platforms use the same image for all nodes
osimage, err = osImage(config)
Expand Down
14 changes: 8 additions & 6 deletions pkg/asset/rhcos/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ func (i *Image) Generate(p asset.Parents) error {
}

func osImage(config *types.InstallConfig) (string, error) {
arch := config.ControlPlane.Architecture

var osimage string
var err error
ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second)
Expand All @@ -73,15 +75,15 @@ func osImage(config *types.InstallConfig) (string, error) {
osimage = config.Platform.AWS.AMIID
break
}
osimage, err = rhcos.AMI(ctx, config.Platform.AWS.Region)
osimage, err = rhcos.AMI(ctx, arch, config.Platform.AWS.Region)
case gcp.Name:
osimage, err = rhcos.GCP(ctx)
osimage, err = rhcos.GCP(ctx, arch)
case libvirt.Name:
osimage, err = rhcos.QEMU(ctx)
osimage, err = rhcos.QEMU(ctx, arch)
case openstack.Name, ovirt.Name:
osimage, err = rhcos.OpenStack(ctx)
osimage, err = rhcos.OpenStack(ctx, arch)
case azure.Name:
osimage, err = rhcos.VHD(ctx)
osimage, err = rhcos.VHD(ctx, arch)
case baremetal.Name:
// Check for RHCOS image URL override
if oi := config.Platform.BareMetal.ClusterOSImage; oi != "" {
Expand All @@ -92,7 +94,7 @@ func osImage(config *types.InstallConfig) (string, error) {
// Note that baremetal IPI currently uses the OpenStack image
// because this contains the necessary ironic config drive
// ignition support, which isn't enabled in the UPI BM images
osimage, err = rhcos.OpenStack(ctx)
osimage, err = rhcos.OpenStack(ctx, arch)
case none.Name, vsphere.Name:

default:
Expand Down
6 changes: 4 additions & 2 deletions pkg/rhcos/ami.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import (
"context"

"github.com/pkg/errors"

"github.com/openshift/installer/pkg/types"
)

// AMI fetches the HVM AMI ID of the Red Hat Enterprise Linux CoreOS release.
func AMI(ctx context.Context, region string) (string, error) {
meta, err := fetchRHCOSBuild(ctx)
func AMI(ctx context.Context, arch types.Architecture, region string) (string, error) {
meta, err := fetchRHCOSBuild(ctx, arch)
if err != nil {
return "", errors.Wrap(err, "failed to fetch RHCOS metadata")
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/rhcos/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import (
"context"

"github.com/pkg/errors"

"github.com/openshift/installer/pkg/types"
)

// VHD fetches the URL of the public Azure storage bucket containing the RHCOS image
func VHD(ctx context.Context) (string, error) {
meta, err := fetchRHCOSBuild(ctx)
func VHD(ctx context.Context, arch types.Architecture) (string, error) {
meta, err := fetchRHCOSBuild(ctx, arch)
if err != nil {
return "", errors.Wrap(err, "failed to fetch RHCOS metadata")
}
Expand Down
16 changes: 13 additions & 3 deletions pkg/rhcos/builds.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,18 @@ package rhcos
import (
"context"
"encoding/json"
"fmt"
"io/ioutil"
"os"

"github.com/openshift/installer/data"
"github.com/pkg/errors"

"github.com/openshift/installer/pkg/types"
)

var (
errInvalidArch = fmt.Errorf("no build metadata for given architecture")
)

type metadata struct {
Expand Down Expand Up @@ -37,15 +45,17 @@ type metadata struct {
OSTreeVersion string `json:"ostree-version"`
}

func fetchRHCOSBuild(ctx context.Context) (*metadata, error) {
file, err := data.Assets.Open("rhcos.json")
func fetchRHCOSBuild(ctx context.Context, arch types.Architecture) (*metadata, error) {
file, err := data.Assets.Open(fmt.Sprintf("rhcos-%s.json", arch))
if err != nil {
return nil, err
}
defer file.Close()

body, err := ioutil.ReadAll(file)
if err != nil {
if os.IsNotExist(err) {
return nil, errInvalidArch
} else if err != nil {
return nil, err
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/rhcos/gcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import (
"context"

"github.com/pkg/errors"

"github.com/openshift/installer/pkg/types"
)

// GCP fetches the URL of the public GCP storage bucket containing the RHCOS image
func GCP(ctx context.Context) (string, error) {
meta, err := fetchRHCOSBuild(ctx)
func GCP(ctx context.Context, arch types.Architecture) (string, error) {
meta, err := fetchRHCOSBuild(ctx, arch)
if err != nil {
return "", errors.Wrap(err, "failed to fetch RHCOS metadata")
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/rhcos/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ import (
"net/url"

"github.com/pkg/errors"

"github.com/openshift/installer/pkg/types"
)

// OpenStack fetches the URL of the Red Hat Enterprise Linux CoreOS release,
// for the openstack platform
func OpenStack(ctx context.Context) (string, error) {
meta, err := fetchRHCOSBuild(ctx)
func OpenStack(ctx context.Context, arch types.Architecture) (string, error) {
meta, err := fetchRHCOSBuild(ctx, arch)
if err != nil {
return "", errors.Wrap(err, "failed to fetch RHCOS metadata")
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/rhcos/qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import (
"net/url"

"github.com/pkg/errors"

"github.com/openshift/installer/pkg/types"
)

// QEMU fetches the URL of the Red Hat Enterprise Linux CoreOS release.
func QEMU(ctx context.Context) (string, error) {
meta, err := fetchRHCOSBuild(ctx)
func QEMU(ctx context.Context, arch types.Architecture) (string, error) {
meta, err := fetchRHCOSBuild(ctx, arch)
if err != nil {
return "", errors.Wrap(err, "failed to fetch RHCOS metadata")
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/types/defaults/machinepools.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@ func SetMachinePoolDefaults(p *types.MachinePool, platform string) {
if p.Hyperthreading == "" {
p.Hyperthreading = types.HyperthreadingEnabled
}
if p.Architecture == "" {
p.Architecture = types.ArchitectureAMD64
}
}
1 change: 1 addition & 0 deletions pkg/types/defaults/machinepools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func defaultMachinePool(name string) *types.MachinePool {
Name: name,
Replicas: pointer.Int64Ptr(3),
Hyperthreading: types.HyperthreadingEnabled,
Architecture: types.ArchitectureAMD64,
}
}

Expand Down
12 changes: 12 additions & 0 deletions pkg/types/machinepools.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ const (
HyperthreadingDisabled HyperthreadingMode = "Disabled"
)

// Architecture is the instruction set architecture for the machines in a pool.
type Architecture string

const (
// ArchitectureAMD64 indicates AMD64 (x86_64).
ArchitectureAMD64 = "amd64"
)

// MachinePool is a pool of machines to be installed.
type MachinePool struct {
// Name is the name of the machine pool.
Expand All @@ -39,6 +47,10 @@ type MachinePool struct {
// +optional
// Default is for hyperthreading to be enabled.
Hyperthreading HyperthreadingMode `json:"hyperthreading,omitempty"`

// Architecture is the instruction set architecture of the machine pool.
// Defaults to amd64.
Architecture Architecture `json:"architecture,omitempty"`
}

// MachinePoolPlatform is the platform-specific configuration for a machine
Expand Down
7 changes: 5 additions & 2 deletions pkg/types/validation/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func ValidateInstallConfig(c *types.InstallConfig, openStackValidValuesFetcher o
} else {
allErrs = append(allErrs, field.Required(field.NewPath("controlPlane"), "controlPlane is required"))
}
allErrs = append(allErrs, validateCompute(&c.Platform, c.Compute, field.NewPath("compute"))...)
allErrs = append(allErrs, validateCompute(&c.Platform, c.ControlPlane, c.Compute, field.NewPath("compute"))...)
if err := validate.ImagePullSecret(c.PullSecret); err != nil {
allErrs = append(allErrs, field.Invalid(field.NewPath("pullSecret"), c.PullSecret, err.Error()))
}
Expand Down Expand Up @@ -310,7 +310,7 @@ func validateControlPlane(platform *types.Platform, pool *types.MachinePool, fld
return allErrs
}

func validateCompute(platform *types.Platform, pools []types.MachinePool, fldPath *field.Path) field.ErrorList {
func validateCompute(platform *types.Platform, control *types.MachinePool, pools []types.MachinePool, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
poolNames := map[string]bool{}
for i, p := range pools {
Expand All @@ -322,6 +322,9 @@ func validateCompute(platform *types.Platform, pools []types.MachinePool, fldPat
allErrs = append(allErrs, field.Duplicate(poolFldPath.Child("name"), p.Name))
}
poolNames[p.Name] = true
if control != nil && control.Architecture != p.Architecture {
allErrs = append(allErrs, field.Invalid(poolFldPath.Child("architecture"), p.Architecture, "heteregeneous multi-arch is not supported; compute pool architecture must match control plane"))
}
allErrs = append(allErrs, ValidateMachinePool(platform, &p, poolFldPath)...)
}
return allErrs
Expand Down
1 change: 1 addition & 0 deletions pkg/types/validation/installconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,6 +904,7 @@ func TestValidateInstallConfig(t *testing.T) {
return c
}(),
},
// TODO(crawford): add a test to validate that homogeneous clusters are enforced once an additional architecture is added
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
15 changes: 15 additions & 0 deletions pkg/types/validation/machinepools.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,18 @@ var (
}
return v
}()

validArchitectures = map[types.Architecture]bool{
types.ArchitectureAMD64: true,
}

validArchitectureValues = func() []string {
v := make([]string, 0, len(validArchitectures))
for m := range validArchitectures {
v = append(v, string(m))
}
return v
}()
)

// ValidateMachinePool checks that the specified machine pool is valid.
Expand All @@ -46,6 +58,9 @@ func ValidateMachinePool(platform *types.Platform, p *types.MachinePool, fldPath
if !validHyperthreadingModes[p.Hyperthreading] {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("hyperthreading"), p.Hyperthreading, validHyperthreadingModeValues))
}
if !validArchitectures[p.Architecture] {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("architecture"), p.Architecture, validArchitectureValues))
}
allErrs = append(allErrs, validateMachinePoolPlatform(platform, &p.Platform, fldPath.Child("platform"))...)
return allErrs
}
Expand Down
1 change: 1 addition & 0 deletions pkg/types/validation/machinepools_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ func validMachinePool(name string) *types.MachinePool {
Name: name,
Replicas: pointer.Int64Ptr(1),
Hyperthreading: types.HyperthreadingDisabled,
Architecture: types.ArchitectureAMD64,
}
}

Expand Down

0 comments on commit 2583ba4

Please sign in to comment.