Skip to content
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
23 changes: 1 addition & 22 deletions data/data/gcp/cluster/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ locals {
worker_subnet_cidr = cidrsubnet(var.machine_v4_cidrs[0], 1, 1) #worker subnet is a smaller subnet within the vnet. e.g., from /21 to /22
public_endpoints = var.gcp_publish_strategy == "External" ? true : false

gcp_image = var.gcp_preexisting_image ? var.gcp_image : google_compute_image.cluster[0].self_link
gcp_image = var.gcp_image
description = "Created By OpenShift Installer"
}

Expand Down Expand Up @@ -81,24 +81,3 @@ module "dns" {
gcp_extra_labels = var.gcp_extra_labels
}

resource "google_compute_image" "cluster" {
count = var.gcp_preexisting_image ? 0 : 1
description = local.description

name = "${var.cluster_id}-rhcos-image"

# See https://github.com/openshift/installer/issues/2546
guest_os_features {
type = "SECURE_BOOT"
}
guest_os_features {
type = "UEFI_COMPATIBLE"
}

raw_disk {
source = var.gcp_image_uri
}

licenses = var.gcp_image_licenses
labels = var.gcp_extra_labels
}
17 changes: 0 additions & 17 deletions data/data/gcp/variables-gcp.tf
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,6 @@ variable "gcp_master_instance_type" {
description = "Instance type for the master node(s). Example: `n1-standard-4`"
}

variable "gcp_image_uri" {
type = string
description = "URL to Raw Image for all nodes. This is used in case a new image needs to be generated for the nodes."
}

variable "gcp_image" {
type = string
description = "URL to the Image for all nodes."
Expand All @@ -69,12 +64,6 @@ variable "gcp_instance_service_account" {
default = ""
}

variable "gcp_preexisting_image" {
type = bool
default = true
description = "Specifies whether an existing GCP Image should be used or a new one created for installation"
}

variable "gcp_master_root_volume_type" {
type = string
description = "The type of volume for the root block device of master nodes."
Expand Down Expand Up @@ -128,12 +117,6 @@ variable "gcp_publish_strategy" {
description = "The cluster publishing strategy, either Internal or External"
}

variable "gcp_image_licenses" {
type = list(string)
description = "The licenses to use when creating compute instances"
default = []
}

variable "gcp_root_volume_kms_key_link" {
type = string
description = "The GCP self link of KMS key to encrypt the volume."
Expand Down
10 changes: 0 additions & 10 deletions data/data/install.openshift.io_installconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3178,16 +3178,6 @@ spec:
type: string
type: array
type: object
licenses:
description: Licenses is a list of licenses to apply to the compute
images The value should a list of strings (https URLs only)
representing the license keys. When set, this will cause the
installer to copy the image into user's project. This option
is incompatible with any mechanism that makes use of pre-built
images such as the current env OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE
items:
type: string
type: array
network:
description: Network specifies an existing VPC where the cluster
should be created rather than provisioning a new one.
Expand Down
7 changes: 1 addition & 6 deletions pkg/asset/cluster/tfvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -490,18 +490,13 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
if img == nil {
return fmt.Errorf("%s: No GCP build found", st.FormatPrefix(archName))
}
// For backwards compatibility, we generate this URL to the image (only applies to RHCOS, not FCOS/OKD)
// right now. It will only be used if nested virt or other licenses are enabled, which we
// really should deprecate and remove - xref https://github.com/openshift/installer/pull/4696
imageURL := fmt.Sprintf("https://storage.googleapis.com/rhcos/rhcos/%s.tar.gz", img.Name)

data, err := gcptfvars.TFVars(
gcptfvars.TFVarsSources{
Auth: auth,
MasterConfigs: masterConfigs,
WorkerConfigs: workerConfigs,
CreateFirewallRules: createFirewallRules,
ImageURI: imageURL,
ImageLicenses: installConfig.Config.GCP.Licenses,
PreexistingNetwork: preexistingnetwork,
PublicZoneName: publicZoneName,
PrivateZoneName: privateZoneName,
Expand Down
4 changes: 1 addition & 3 deletions pkg/asset/machines/gcp/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,7 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine

func provider(clusterID string, platform *gcp.Platform, mpool *gcp.MachinePool, osImage string, azIdx int, role, userDataSecret string, credentialsMode types.CredentialsMode) (*machineapi.GCPMachineProviderSpec, error) {
az := mpool.Zones[azIdx]
if len(platform.Licenses) > 0 {
osImage = fmt.Sprintf("%s-rhcos-image", clusterID)
} else if mpool.OSImage != nil {
if mpool.OSImage != nil {
osImage = fmt.Sprintf("projects/%s/global/images/%s", mpool.OSImage.Project, mpool.OSImage.Name)
}
Comment on lines 135 to 137
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: would it be cleaner to do this assignment when calling provider so that the osImage argument is already the correct value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Then I think we have more duplicated code

network, subnetwork, err := getNetworks(platform, clusterID, role)
Expand Down
12 changes: 0 additions & 12 deletions pkg/tfvars/gcp/gcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,8 @@ type config struct {
CreateFirewallRules bool `json:"gcp_create_firewall_rules"`
MasterInstanceType string `json:"gcp_master_instance_type,omitempty"`
MasterAvailabilityZones []string `json:"gcp_master_availability_zones"`
ImageURI string `json:"gcp_image_uri,omitempty"`
Image string `json:"gcp_image,omitempty"`
PreexistingImage bool `json:"gcp_preexisting_image"`
InstanceServiceAccount string `json:"gcp_instance_service_account,omitempty"`
ImageLicenses []string `json:"gcp_image_licenses,omitempty"`
VolumeType string `json:"gcp_master_root_volume_type"`
VolumeSize int64 `json:"gcp_master_root_volume_size"`
VolumeKMSKeyLink string `json:"gcp_root_volume_kms_key_link"`
Expand All @@ -59,8 +56,6 @@ type config struct {
type TFVarsSources struct {
Auth Auth
CreateFirewallRules bool
ImageURI string
ImageLicenses []string
MasterConfigs []*machineapi.GCPMachineProviderSpec
WorkerConfigs []*machineapi.GCPMachineProviderSpec
PublicZoneName string
Expand Down Expand Up @@ -95,9 +90,7 @@ func TFVars(sources TFVarsSources) ([]byte, error) {
MasterAvailabilityZones: masterAvailabilityZones,
VolumeType: masterConfig.Disks[0].Type,
VolumeSize: masterConfig.Disks[0].SizeGB,
ImageURI: sources.ImageURI,
Image: masterConfig.Disks[0].Image,
ImageLicenses: sources.ImageLicenses,
PublicZoneName: sources.PublicZoneName,
PrivateZoneName: sources.PrivateZoneName,
PublishStrategy: string(sources.PublishStrategy),
Expand All @@ -112,11 +105,6 @@ func TFVars(sources TFVarsSources) ([]byte, error) {
ExtraLabels: labels,
}

cfg.PreexistingImage = true
if len(sources.ImageLicenses) > 0 {
cfg.PreexistingImage = false
}

if masterConfig.Disks[0].EncryptionKey != nil {
cfg.VolumeKMSKeyLink = generateDiskEncryptionKeyLink(masterConfig.Disks[0].EncryptionKey, masterConfig.ProjectID)
}
Expand Down
8 changes: 0 additions & 8 deletions pkg/types/gcp/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,6 @@ type Platform struct {
// +optional
ComputeSubnet string `json:"computeSubnet,omitempty"`

// Licenses is a list of licenses to apply to the compute images
// The value should a list of strings (https URLs only) representing the license keys.
// When set, this will cause the installer to copy the image into user's project.
// This option is incompatible with any mechanism that makes use of pre-built images
// such as the current env OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE
// +optional
Licenses []string `json:"licenses,omitempty"`

// userLabels has additional keys and values that the installer will add as
// labels to all resources that it creates on GCP. Resources created by the
// cluster itself may not include these labels. This is a TechPreview feature
Expand Down
12 changes: 0 additions & 12 deletions pkg/types/gcp/validation/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@ package validation

import (
"fmt"
"os"
"regexp"
"sort"

"k8s.io/apimachinery/pkg/util/validation/field"

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

var (
Expand Down Expand Up @@ -127,16 +125,6 @@ func ValidatePlatform(p *gcp.Platform, fldPath *field.Path, ic *types.InstallCon
allErrs = append(allErrs, field.Required(fldPath.Child("network"), "must provide a VPC network when supplying subnets"))
}

if oi, ok := os.LookupEnv("OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE"); ok && oi != "" && len(p.Licenses) > 0 {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("licenses"), "the use of custom image licenses is forbidden if an OPENSHIFT_INSTALL_OS_IMAGE_OVERRIDE is specified"))
}

for i, license := range p.Licenses {
if validate.URIWithProtocol(license, "https") != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("licenses").Index(i), license, "licenses must be URLs (https) only"))
}
}

// check if configured userLabels are valid.
allErrs = append(allErrs, validateUserLabels(p.UserLabels, fldPath.Child("userLabels"))...)

Expand Down