Skip to content
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.

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

OpenStack: automatically populate RHCOS image #2473

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions Gopkg.lock

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

7 changes: 1 addition & 6 deletions data/data/openstack/bootstrap/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -109,19 +109,14 @@ EOF
}
}

data "openstack_images_image_v2" "bootstrap_image" {
name = var.image_name
most_recent = true
}

data "openstack_compute_flavor_v2" "bootstrap_flavor" {
name = var.flavor_name
}

resource "openstack_compute_instance_v2" "bootstrap" {
name = "${var.cluster_id}-bootstrap"
flavor_id = data.openstack_compute_flavor_v2.bootstrap_flavor.id
image_id = data.openstack_images_image_v2.bootstrap_image.id
image_id = var.base_image_id

user_data = data.ignition_config.redirect.rendered

Expand Down
4 changes: 2 additions & 2 deletions data/data/openstack/bootstrap/variables.tf
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
variable "image_name" {
variable "base_image_id" {
type = string
description = "The name of the Glance image for the bootstrap node."
description = "The identifier of the Glance image for the bootstrap node."
}

variable "extra_tags" {
Expand Down
22 changes: 20 additions & 2 deletions data/data/openstack/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ module "bootstrap" {

cluster_id = var.cluster_id
extra_tags = var.openstack_extra_tags
image_name = var.openstack_base_image
base_image_id = data.openstack_images_image_v2.base_image.id
flavor_name = var.openstack_master_flavor_name
ignition = var.ignition_bootstrap
api_int_ip = var.openstack_api_int_ip
Expand All @@ -42,7 +42,7 @@ module "bootstrap" {
module "masters" {
source = "./masters"

base_image = var.openstack_base_image
base_image_id = data.openstack_images_image_v2.base_image.id
cluster_id = var.cluster_id
flavor_name = var.openstack_master_flavor_name
instance_count = var.master_count
Expand Down Expand Up @@ -72,3 +72,21 @@ module "topology" {
trunk_support = var.openstack_trunk_support
octavia_support = var.openstack_octavia_support
}

resource "openstack_images_image_v2" "base_image" {
// we need to create a new image only if the base image url has been provided, plus base image name is <cluster_id>-rhcos
count = var.openstack_base_image_url != "" && var.openstack_base_image_name == "${var.cluster_id}-rhcos" ? 1 : 0
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be simplified to var.openstack_base_image_url != "" right?

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather leave it like that, just in case the user exports TF_VAR_openstack_base_image_url. We've had an issue in our CI where a previous version of this patch created a duplicate rhcos image, it caused all the jobs to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is an optional check to prevent possible regressions that can break our ci


name = var.openstack_base_image_name
image_source_url = var.openstack_base_image_url
container_format = "bare"
disk_format = "qcow2"

tags = ["openshiftClusterID=${var.cluster_id}"]
}

data "openstack_images_image_v2" "base_image" {
name = var.openstack_base_image_name

depends_on = ["openstack_images_image_v2.base_image"]
}
9 changes: 2 additions & 7 deletions data/data/openstack/masters/main.tf
Original file line number Diff line number Diff line change
@@ -1,8 +1,3 @@
data "openstack_images_image_v2" "masters_img" {
name = var.base_image
most_recent = true
}

data "openstack_compute_flavor_v2" "masters_flavor" {
name = var.flavor_name
}
Expand Down Expand Up @@ -38,15 +33,15 @@ resource "openstack_blockstorage_volume_v3" "master_volume" {

size = var.root_volume_size
volume_type = var.root_volume_type
image_id = data.openstack_images_image_v2.masters_img.id
image_id = var.base_image_id
}

resource "openstack_compute_instance_v2" "master_conf" {
name = "${var.cluster_id}-master-${count.index}"
count = var.instance_count

flavor_id = data.openstack_compute_flavor_v2.masters_flavor.id
image_id = var.root_volume_size == null ? data.openstack_images_image_v2.masters_img.id : null
image_id = var.root_volume_size == null ? var.base_image_id : null
security_groups = var.master_sg_ids
user_data = element(
data.ignition_config.master_ignition_config.*.rendered,
Expand Down
5 changes: 3 additions & 2 deletions data/data/openstack/masters/variables.tf
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
variable "base_image" {
type = string
variable "base_image_id" {
type = string
description = "The identifier of the Glance image for master nodes."
}

variable "cluster_id" {
Expand Down
9 changes: 7 additions & 2 deletions data/data/openstack/variables-openstack.tf
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,17 @@ variable "openstack_master_root_volume_size" {
description = "The size of the volume in gigabytes for the root block device of master nodes."
}

variable "openstack_base_image" {
variable "openstack_base_image_name" {
type = string
default = "rhcos"
description = "Name of the base image to use for the nodes."
Fedosin marked this conversation as resolved.
Show resolved Hide resolved
}

variable "openstack_base_image_url" {
type = string
Copy link
Member

Choose a reason for hiding this comment

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

We need a default value, otherwise it fails with The input variable "openstack_base_image_url" has not been assigned a value. when exporting OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Funny thing, I did that but forgot to git commit --amend it and pushed the patch without this change. Agree, this is important.

default = ""
description = "URL of the base image to use for the nodes."
}

variable "openstack_credentials_auth_url" {
type = string
default = ""
Expand Down
2 changes: 2 additions & 0 deletions pkg/asset/cluster/tfvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
ingressVIP.String(),
installConfig.Config.Platform.OpenStack.TrunkSupport,
installConfig.Config.Platform.OpenStack.OctaviaSupport,
string(*rhcosImage),
clusterID.InfraID,
)
if err != nil {
return errors.Wrapf(err, "failed to get %s Terraform variables", platform)
Expand Down
6 changes: 5 additions & 1 deletion pkg/asset/machines/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/openshift/installer/pkg/asset/machines/machineconfig"
"github.com/openshift/installer/pkg/asset/machines/openstack"
"github.com/openshift/installer/pkg/asset/rhcos"
rhcosutils "github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/types"
awstypes "github.com/openshift/installer/pkg/types/aws"
awsdefaults "github.com/openshift/installer/pkg/types/aws/defaults"
Expand Down Expand Up @@ -181,7 +182,10 @@ func (m *Master) Generate(dependencies asset.Parents) error {
mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform)
mpool.Set(pool.Platform.OpenStack)
pool.Platform.OpenStack = &mpool
machines, err = openstack.Machines(clusterID.InfraID, ic, pool, string(*rhcosImage), "master", "master-user-data")

imageName, _ := rhcosutils.GenerateOpenStackImageName(string(*rhcosImage), clusterID.InfraID)

machines, err = openstack.Machines(clusterID.InfraID, ic, pool, imageName, "master", "master-user-data")
if err != nil {
return errors.Wrap(err, "failed to create master machine objects")
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/asset/machines/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/openshift/installer/pkg/asset/machines/machineconfig"
"github.com/openshift/installer/pkg/asset/machines/openstack"
"github.com/openshift/installer/pkg/asset/rhcos"
rhcosutils "github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/types"
awstypes "github.com/openshift/installer/pkg/types/aws"
awsdefaults "github.com/openshift/installer/pkg/types/aws/defaults"
Expand Down Expand Up @@ -247,7 +248,9 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
mpool.Set(pool.Platform.OpenStack)
pool.Platform.OpenStack = &mpool

sets, err := openstack.MachineSets(clusterID.InfraID, ic, &pool, string(*rhcosImage), "worker", "worker-user-data")
imageName, _ := rhcosutils.GenerateOpenStackImageName(string(*rhcosImage), clusterID.InfraID)

sets, err := openstack.MachineSets(clusterID.InfraID, ic, &pool, imageName, "worker", "worker-user-data")
if err != nil {
return errors.Wrap(err, "failed to create master machine objects")
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/asset/rhcos/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func osImage(config *types.InstallConfig) (string, error) {
case libvirt.Name:
osimage, err = rhcos.QEMU(ctx)
case openstack.Name:
osimage = "rhcos"
osimage, err = rhcos.OpenStack(ctx)
case azure.Name:
osimage, err = rhcos.VHD(ctx)
case baremetal.Name:
Expand Down
40 changes: 40 additions & 0 deletions pkg/destroy/openstack/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/gophercloud/gophercloud"
"github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes"
"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
"github.com/gophercloud/gophercloud/openstack/imageservice/v2/images"
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/apiversions"
"github.com/gophercloud/gophercloud/openstack/loadbalancer/v2/loadbalancers"
"github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
Expand Down Expand Up @@ -133,6 +134,7 @@ func populateDeleteFuncs(funcs map[string]deleteFunc) {
funcs["deleteContainers"] = deleteContainers
funcs["deleteVolumes"] = deleteVolumes
funcs["deleteFloatingIPs"] = deleteFloatingIPs
funcs["deleteImages"] = deleteImages
}

// filterObjects will do client-side filtering given an appropriately filled out
Expand Down Expand Up @@ -876,3 +878,41 @@ func deleteFloatingIPs(opts *clientconfig.ClientOpts, filter Filter, logger logr
}
return len(allFloatingIPs) == 0, nil
}

func deleteImages(opts *clientconfig.ClientOpts, filter Filter, logger logrus.FieldLogger) (bool, error) {
logger.Debug("Deleting openstack base image")
defer logger.Debugf("Exiting deleting openstack base image")

conn, err := clientconfig.NewServiceClient("image", opts)
if err != nil {
logger.Fatalf("%v", err)
os.Exit(1)
}

listOpts := images.ListOpts{
Tags: filterTags(filter),
}

allPages, err := images.List(conn, listOpts).AllPages()
if err != nil {
logger.Fatalf("%v", err)
os.Exit(1)
}

allImages, err := images.ExtractImages(allPages)
if err != nil {
logger.Fatalf("%v", err)
os.Exit(1)
}

for _, image := range allImages {
logger.Debugf("Deleting image: %+v", image.ID)
err := images.Delete(conn, image.ID).ExtractErr()
if err != nil {
// This can fail if the image is still in use by other VMs
logger.Debugf("Deleting Image failed: %v", err)
return false, nil
}
}
return true, nil
}
14 changes: 14 additions & 0 deletions pkg/rhcos/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,17 @@ func OpenStack(ctx context.Context) (string, error) {

return base.ResolveReference(relOpenStack).String(), nil
}

// GenerateOpenStackImageName returns Glance image name for instances.
func GenerateOpenStackImageName(rhcosImage, infraID string) (imageName string, isURL bool) {
// Here we check whether rhcosImage is a URL or not. If this is the first case, it means that Glance image
// should be created by the installer with the universal name "<infraID>-rhcos". Otherwise, it means
// that we are given the name of the pre-created Glance image, which the installer should use for node
// provisioning.
_, err := url.ParseRequestURI(rhcosImage)
if err != nil {
return rhcosImage, false
}

return infraID + "-rhcos", true
}
25 changes: 22 additions & 3 deletions pkg/tfvars/openstack/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ package openstack
import (
"encoding/json"

"github.com/openshift/installer/pkg/rhcos"

"sigs.k8s.io/cluster-api-provider-openstack/pkg/apis/openstackproviderconfig/v1alpha1"
)

type config struct {
BaseImage string `json:"openstack_base_image,omitempty"`
BaseImageName string `json:"openstack_base_image_name,omitempty"`
BaseImageURL string `json:"openstack_base_image_url,omitempty"`
ExternalNetwork string `json:"openstack_external_network,omitempty"`
Cloud string `json:"openstack_credentials_cloud,omitempty"`
FlavorName string `json:"openstack_master_flavor_name,omitempty"`
Expand All @@ -23,9 +26,8 @@ type config struct {
}

// TFVars generates OpenStack-specific Terraform variables.
func TFVars(masterConfig *v1alpha1.OpenstackProviderSpec, cloud string, externalNetwork string, lbFloatingIP string, apiVIP string, dnsVIP string, ingressVIP string, trunkSupport string, octaviaSupport string) ([]byte, error) {
func TFVars(masterConfig *v1alpha1.OpenstackProviderSpec, cloud string, externalNetwork string, lbFloatingIP string, apiVIP string, dnsVIP string, ingressVIP string, trunkSupport string, octaviaSupport string, baseImage string, infraID string) ([]byte, error) {
cfg := &config{
BaseImage: masterConfig.Image,
ExternalNetwork: externalNetwork,
Cloud: cloud,
FlavorName: masterConfig.Flavor,
Expand All @@ -36,6 +38,23 @@ func TFVars(masterConfig *v1alpha1.OpenstackProviderSpec, cloud string, external
TrunkSupport: trunkSupport,
OctaviaSupport: octaviaSupport,
}

// Normally baseImage contains a URL that we will use to create a new Glance image, but for testing
// purposes we also allow to set a custom Glance image name to skip the uploading. Here we check
// whether baseImage is a URL or not. If this is the first case, it means that the image should be
// created by the installer from the URL. Otherwise, it means that we are given the name of the pre-created
// Glance image, which we should use for instances.
imageName, isURL := rhcos.GenerateOpenStackImageName(baseImage, infraID)
cfg.BaseImageName = imageName
if isURL {
// Valid URL -> use baseImage as a URL that will be used to create new Glance image with name "<infraID>-rhcos".
cfg.BaseImageURL = baseImage
} else {
// Not a URL -> use baseImage value as a Glance image name.

// TODO(mfedosin): add validations that this image exists and there are no other images with this name.
}

if masterConfig.RootVolume != nil {
cfg.RootVolumeSize = masterConfig.RootVolume.Size
cfg.RootVolumeType = masterConfig.RootVolume.VolumeType
Expand Down
1 change: 1 addition & 0 deletions vendor/github.com/gophercloud/gophercloud/internal/pkg.go

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

34 changes: 34 additions & 0 deletions vendor/github.com/gophercloud/gophercloud/internal/util.go

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

Loading