Skip to content

drop capability to specify image in install-config #1052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 7 commits into from
Jan 12, 2019
Merged

drop capability to specify image in install-config #1052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ff9d8f2
types/libvirt: remove per-machine-pool configuration
staebler Dec 6, 2018
2bd6c9d
asset: add rhcos Image asset to generate image location
abhinavdahiya Jan 11, 2019
3eb958b
pkg: use rhcos Image to fetch ami for AWS
abhinavdahiya Jan 11, 2019
94d9958
asset/installconfig: drop image related code paths
abhinavdahiya Jan 11, 2019
255566f
types/openstack: drop GetImageNames method
abhinavdahiya Jan 11, 2019
b07f23e
types: drop image options from install config for all platforms
abhinavdahiya Jan 11, 2019
38e8cad
docs: update the resource dep graph
abhinavdahiya Jan 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions Gopkg.lock
View file
Open in desktop

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

940 changes: 467 additions & 473 deletions docs/design/resource_dep.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
8 changes: 5 additions & 3 deletions pkg/asset/cluster/tfvars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/openshift/installer/pkg/asset/ignition/bootstrap"
"github.com/openshift/installer/pkg/asset/ignition/machine"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/rhcos"
"github.com/openshift/installer/pkg/tfvars"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -34,6 +35,7 @@ func (t *TerraformVariables) Name() string {
func (t *TerraformVariables) Dependencies() []asset.Asset {
return []asset.Asset{
&installconfig.InstallConfig{},
new(rhcos.Image),
Copy link
Member

@wking wking Jan 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not &rhcos.Image{} like its neighbors? And the same elsewhere in this commit:

$ git show d65693719 | grep 'new('
+		new(rhcos.Image),
+	rhcosImage := new(rhcos.Image)
+		new(rhcos.Image),
+	rhcosImage := new(rhcos.Image)
+		new(rhcos.Image),
+	rhcosImage := new(rhcos.Image)

Copy link
Contributor Author

@abhinavdahiya abhinavdahiya Jan 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://play.golang.org/p/NUhwmraqAtM

prog.go:10:7: invalid pointer type *Image for composite literal

Copy link
Contributor Author

@abhinavdahiya abhinavdahiya Jan 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://golang.org/ref/spec#Address_operators

&bootstrap.Bootstrap{},
&machine.Master{},
}
Expand All @@ -44,13 +46,13 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
installConfig := &installconfig.InstallConfig{}
bootstrap := &bootstrap.Bootstrap{}
master := &machine.Master{}
parents.Get(installConfig, bootstrap, master)
rhcosImage := new(rhcos.Image)
parents.Get(installConfig, bootstrap, master, rhcosImage)

bootstrapIgn := string(bootstrap.Files()[0].Data)

masterIgn := string(master.Files()[0].Data)

data, err := tfvars.TFVars(installConfig.Config, bootstrapIgn, masterIgn)
data, err := tfvars.TFVars(installConfig.Config, string(*rhcosImage), bootstrapIgn, masterIgn)
if err != nil {
return errors.Wrap(err, "failed to get Tfvars")
}
Expand Down
12 changes: 0 additions & 12 deletions pkg/asset/installconfig/libvirt/libvirt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,9 @@
package libvirt

import (
"context"

"github.com/pkg/errors"
survey "gopkg.in/AlecAivazis/survey.v1"

"github.com/openshift/installer/pkg/ipnet"
"github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/types/libvirt"
"github.com/openshift/installer/pkg/validate"
)
Expand Down Expand Up @@ -40,18 +36,10 @@ func Platform() (*libvirt.Platform, error) {
return nil, err
}

qcowImage, err := rhcos.QEMU(context.TODO(), rhcos.DefaultChannel)
if err != nil {
return nil, errors.Wrap(err, "failed to fetch QEMU image URL")
}

return &libvirt.Platform{
Network: libvirt.Network{
IfName: defaultNetworkIfName,
},
DefaultMachinePlatform: &libvirt.MachinePool{
Image: qcowImage,
},
URI: uri,
}, nil
}
Expand Down
29 changes: 0 additions & 29 deletions pkg/asset/installconfig/openstack/openstack.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,34 +72,6 @@ func Platform() (*openstack.Platform, error) {
return nil, err
}

imageNames, err := validValuesFetcher.GetImageNames(cloud)
if err != nil {
return nil, err
}
sort.Strings(imageNames)
var image string
err = survey.Ask([]*survey.Question{
{
Prompt: &survey.Select{
Message: "Image",
Help: "The OpenStack image name to be used for installation.",
Default: "rhcos",
Options: imageNames,
},
Validate: survey.ComposeValidators(survey.Required, func(ans interface{}) error {
value := ans.(string)
i := sort.SearchStrings(imageNames, value)
if i == len(imageNames) || imageNames[i] != value {
return errors.Errorf("invalid image name %q, should be one of %+v", value, strings.Join(imageNames, ", "))
}
return nil
}),
},
}, &image)
if err != nil {
return nil, err
}

networkNames, err := validValuesFetcher.GetNetworkNames(cloud)
if err != nil {
return nil, err
Expand Down Expand Up @@ -169,7 +141,6 @@ func Platform() (*openstack.Platform, error) {

return &openstack.Platform{
Region: region,
BaseImage: image,
Cloud: cloud,
ExternalNetwork: extNet,
FlavorName: flavor,
Expand Down
9 changes: 5 additions & 4 deletions pkg/asset/machines/aws/machines.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

// Machines returns a list of machines for a machinepool.
func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDataSecret string) ([]clusterapi.Machine, error) {
func Machines(config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string) ([]clusterapi.Machine, error) {
if configPlatform := config.Platform.Name(); configPlatform != aws.Name {
return nil, fmt.Errorf("non-AWS configuration: %q", configPlatform)
}
Expand All @@ -37,7 +37,7 @@ func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDa
var machines []clusterapi.Machine
for idx := int64(0); idx < total; idx++ {
azIndex := int(idx) % len(azs)
provider, err := provider(config.ClusterID, clustername, platform, mpool, azIndex, role, userDataSecret)
provider, err := provider(config.ClusterID, clustername, platform, mpool, osImage, azIndex, role, userDataSecret)
if err != nil {
return nil, errors.Wrap(err, "failed to create provider")
}
Expand Down Expand Up @@ -69,8 +69,9 @@ func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDa
return machines, nil
}

func provider(clusterID, clusterName string, platform *aws.Platform, mpool *aws.MachinePool, azIdx int, role, userDataSecret string) (*awsprovider.AWSMachineProviderConfig, error) {
func provider(clusterID, clusterName string, platform *aws.Platform, mpool *aws.MachinePool, osImage string, azIdx int, role, userDataSecret string) (*awsprovider.AWSMachineProviderConfig, error) {
az := mpool.Zones[azIdx]
amiID := osImage
tags, err := tagsFromUserTags(clusterID, clusterName, platform.UserTags)
if err != nil {
return nil, errors.Wrap(err, "failed to create awsprovider.TagSpecifications from UserTags")
Expand All @@ -81,7 +82,7 @@ func provider(clusterID, clusterName string, platform *aws.Platform, mpool *aws.
Kind: "AWSMachineProviderConfig",
},
InstanceType: mpool.InstanceType,
AMI: awsprovider.AWSResourceReference{ID: &mpool.AMIID},
AMI: awsprovider.AWSResourceReference{ID: &amiID},
Tags: tags,
IAMInstanceProfile: &awsprovider.AWSResourceReference{ID: pointer.StringPtr(fmt.Sprintf("%s-%s-profile", clusterName, role))},
UserDataSecret: &corev1.LocalObjectReference{Name: userDataSecret},
Expand Down
4 changes: 2 additions & 2 deletions pkg/asset/machines/aws/machinesets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
)

// MachineSets returns a list of machinesets for a machinepool.
func MachineSets(config *types.InstallConfig, pool *types.MachinePool, role, userDataSecret string) ([]clusterapi.MachineSet, error) {
func MachineSets(config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string) ([]clusterapi.MachineSet, error) {
if configPlatform := config.Platform.Name(); configPlatform != aws.Name {
return nil, fmt.Errorf("non-AWS configuration: %q", configPlatform)
}
Expand All @@ -38,7 +38,7 @@ func MachineSets(config *types.InstallConfig, pool *types.MachinePool, role, use
replicas++
}

provider, err := provider(config.ClusterID, clustername, platform, mpool, idx, role, userDataSecret)
provider, err := provider(config.ClusterID, clustername, platform, mpool, osImage, idx, role, userDataSecret)
if err != nil {
return nil, errors.Wrap(err, "failed to create provider")
}
Expand Down
5 changes: 1 addition & 4 deletions pkg/asset/machines/libvirt/machines.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,11 @@ func Machines(config *types.InstallConfig, pool *types.MachinePool, role, userDa
if configPlatform := config.Platform.Name(); configPlatform != libvirt.Name {
return nil, fmt.Errorf("non-Libvirt configuration: %q", configPlatform)
}
// FIXME: empty is a valid case for Libvirt as we don't use it.
if poolPlatform := pool.Platform.Name(); poolPlatform != "" && poolPlatform != libvirt.Name {
if poolPlatform := pool.Platform.Name(); poolPlatform != libvirt.Name {
return nil, fmt.Errorf("non-Libvirt machine-pool: %q", poolPlatform)
}
clustername := config.ObjectMeta.Name
platform := config.Platform.Libvirt
// FIXME: libvirt actuator does not support any options from machinepool.
// mpool := pool.Platform.Libvirt

total := int64(1)
if pool.Replicas != nil {
Expand Down
25 changes: 10 additions & 15 deletions pkg/asset/machines/master.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package machines

import (
"context"
"fmt"
"time"

"github.com/ghodss/yaml"
"github.com/pkg/errors"
Expand All @@ -17,7 +15,7 @@ import (
"github.com/openshift/installer/pkg/asset/machines/aws"
"github.com/openshift/installer/pkg/asset/machines/libvirt"
"github.com/openshift/installer/pkg/asset/machines/openstack"
"github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/asset/rhcos"
"github.com/openshift/installer/pkg/types"
awstypes "github.com/openshift/installer/pkg/types/aws"
libvirttypes "github.com/openshift/installer/pkg/types/libvirt"
Expand All @@ -43,15 +41,17 @@ func (m *Master) Name() string {
func (m *Master) Dependencies() []asset.Asset {
return []asset.Asset{
&installconfig.InstallConfig{},
new(rhcos.Image),
&machine.Master{},
}
}

// Generate generates the Master asset.
func (m *Master) Generate(dependencies asset.Parents) error {
installconfig := &installconfig.InstallConfig{}
rhcosImage := new(rhcos.Image)
mign := &machine.Master{}
dependencies.Get(installconfig, mign)
dependencies.Get(installconfig, rhcosImage, mign)

var err error
userDataMap := map[string][]byte{"master-user-data": mign.File.Data}
Expand All @@ -67,15 +67,6 @@ func (m *Master) Generate(dependencies asset.Parents) error {
mpool := defaultAWSMachinePoolPlatform()
mpool.Set(ic.Platform.AWS.DefaultMachinePlatform)
mpool.Set(pool.Platform.AWS)
if mpool.AMIID == "" {
ctx, cancel := context.WithTimeout(context.TODO(), 60*time.Second)
defer cancel()
ami, err := rhcos.AMI(ctx, rhcos.DefaultChannel, ic.Platform.AWS.Region)
if err != nil {
return errors.Wrap(err, "failed to determine default AMI")
}
mpool.AMIID = ami
}
if len(mpool.Zones) == 0 {
azs, err := aws.AvailabilityZones(ic.Platform.AWS.Region)
if err != nil {
Expand All @@ -84,7 +75,7 @@ func (m *Master) Generate(dependencies asset.Parents) error {
mpool.Zones = azs
}
pool.Platform.AWS = &mpool
machines, err := aws.Machines(ic, &pool, "master", "master-user-data")
machines, err := aws.Machines(ic, &pool, string(*rhcosImage), "master", "master-user-data")
if err != nil {
return errors.Wrap(err, "failed to create master machine objects")
}
Expand All @@ -97,6 +88,10 @@ func (m *Master) Generate(dependencies asset.Parents) error {
}
m.MachinesRaw = raw
case libvirttypes.Name:
mpool := defaultLibvirtMachinePoolPlatform()
mpool.Set(ic.Platform.Libvirt.DefaultMachinePlatform)
mpool.Set(pool.Platform.Libvirt)
pool.Platform.Libvirt = &mpool
machines, err := libvirt.Machines(ic, &pool, "master", "master-user-data")
if err != nil {
return errors.Wrap(err, "failed to create master machine objects")
Expand All @@ -121,7 +116,7 @@ func (m *Master) Generate(dependencies asset.Parents) error {
config := openstack.MasterConfig{
ClusterName: ic.ObjectMeta.Name,
Instances: instances,
Image: ic.Platform.OpenStack.BaseImage,
Image: string(*rhcosImage),
Region: ic.Platform.OpenStack.Region,
Machine: defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName),
Trunk: trunkSupportBoolean(ic.Platform.OpenStack.TrunkSupport),
Expand Down
29 changes: 14 additions & 15 deletions pkg/asset/machines/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ package machines

import (
"bytes"
"context"
"fmt"
"text/template"
"time"

"github.com/ghodss/yaml"
"github.com/pkg/errors"
Expand All @@ -19,7 +17,7 @@ import (
"github.com/openshift/installer/pkg/asset/machines/aws"
"github.com/openshift/installer/pkg/asset/machines/libvirt"
"github.com/openshift/installer/pkg/asset/machines/openstack"
"github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/asset/rhcos"
"github.com/openshift/installer/pkg/types"
awstypes "github.com/openshift/installer/pkg/types/aws"
libvirttypes "github.com/openshift/installer/pkg/types/libvirt"
Expand All @@ -33,6 +31,10 @@ func defaultAWSMachinePoolPlatform() awstypes.MachinePool {
}
}

func defaultLibvirtMachinePoolPlatform() libvirttypes.MachinePool {
return libvirttypes.MachinePool{}
}

func defaultOpenStackMachinePoolPlatform(flavor string) openstacktypes.MachinePool {
return openstacktypes.MachinePool{
FlavorName: flavor,
Expand Down Expand Up @@ -66,15 +68,17 @@ func (w *Worker) Name() string {
func (w *Worker) Dependencies() []asset.Asset {
return []asset.Asset{
&installconfig.InstallConfig{},
new(rhcos.Image),
&machine.Worker{},
}
}

// Generate generates the Worker asset.
func (w *Worker) Generate(dependencies asset.Parents) error {
installconfig := &installconfig.InstallConfig{}
rhcosImage := new(rhcos.Image)
wign := &machine.Worker{}
dependencies.Get(installconfig, wign)
dependencies.Get(installconfig, rhcosImage, wign)

var err error
userDataMap := map[string][]byte{"worker-user-data": wign.File.Data}
Expand All @@ -90,15 +94,6 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
mpool := defaultAWSMachinePoolPlatform()
mpool.Set(ic.Platform.AWS.DefaultMachinePlatform)
mpool.Set(pool.Platform.AWS)
if mpool.AMIID == "" {
ctx, cancel := context.WithTimeout(context.TODO(), 60*time.Second)
defer cancel()
ami, err := rhcos.AMI(ctx, rhcos.DefaultChannel, ic.Platform.AWS.Region)
if err != nil {
return errors.Wrap(err, "failed to determine default AMI")
}
mpool.AMIID = ami
}
if len(mpool.Zones) == 0 {
azs, err := aws.AvailabilityZones(ic.Platform.AWS.Region)
if err != nil {
Expand All @@ -107,7 +102,7 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
mpool.Zones = azs
}
pool.Platform.AWS = &mpool
sets, err := aws.MachineSets(ic, &pool, "worker", "worker-user-data")
sets, err := aws.MachineSets(ic, &pool, string(*rhcosImage), "worker", "worker-user-data")
if err != nil {
return errors.Wrap(err, "failed to create worker machine objects")
}
Expand All @@ -119,6 +114,10 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
}
w.MachineSetRaw = raw
case libvirttypes.Name:
mpool := defaultLibvirtMachinePoolPlatform()
mpool.Set(ic.Platform.Libvirt.DefaultMachinePlatform)
mpool.Set(pool.Platform.Libvirt)
pool.Platform.Libvirt = &mpool
sets, err := libvirt.MachineSets(ic, &pool, "worker", "worker-user-data")
if err != nil {
return errors.Wrap(err, "failed to create worker machine objects")
Expand All @@ -139,7 +138,7 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
config := openstack.Config{
ClusterName: ic.ObjectMeta.Name,
Replicas: numOfWorkers,
Image: ic.Platform.OpenStack.BaseImage,
Image: string(*rhcosImage),
Region: ic.Platform.OpenStack.Region,
Machine: defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName),
Trunk: trunkSupportBoolean(ic.Platform.OpenStack.TrunkSupport),
Expand Down
Loading