diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index b4b467fbd..84cb12f0f 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -108,6 +108,17 @@ var HypershiftSupportedVersions = HypershiftSupportedVersionsType{} var reBranchVersion = regexp.MustCompile(`^(openshift-|release-)(\d+\.\d+)$`) var reMajorMinorVersion = regexp.MustCompile(`^(\d+)\.(\d+)$`) +// platformProfileSets maps each cloud platform to its cluster-profile set. +// A profile set (e.g. "openshift-org-gcp") is resolved by Test Platform at job +// runtime, which randomly selects one of the underlying "regular" cluster +// profiles. This delegates account dispersement to Test Platform rather than +// ClusterBot querying Boskos and choosing an account itself. See OCPCRT-450. +var platformProfileSets = map[string]string{ + "aws": "openshift-org-aws", + "azure": "openshift-org-azure", + "gcp": "openshift-org-gcp", +} + func (j Job) IsComplete() bool { return j.Complete || len(j.Credentials) > 0 || (len(j.State) > 0 && j.State != prowapiv1.PendingState) } @@ -2221,46 +2232,11 @@ func (m *jobManager) LaunchJobForUser(req *JobRequest) (string, error) { klog.Infof("Job %q requested by user %q with mode %s prow job %s(%s) - params=%s, inputs=%#v", job.Name, req.User, job.Mode, job.JobName, job.BuildCluster, paramsToString(job.JobParams), job.Inputs) - // check what leases are available for platform - if req.Architecture == "amd64" && m.lClient != nil { - switch req.Platform { - case "aws": - metrics1, err := m.lClient.Metrics("aws-quota-slice") - if err != nil { - return "", fmt.Errorf("failed to get metrics for `aws` leases: %v", err) - } - metrics2, err := m.lClient.Metrics("aws-2-quota-slice") - if err != nil { - return "", fmt.Errorf("failed to get metrics for `aws-2` leases: %v", err) - } - if metrics2.Free > metrics1.Free { - job.UseSecondaryAccount = true - } - case "azure": - metrics1, err := m.lClient.Metrics("azure4-quota-slice") - if err != nil { - return "", fmt.Errorf("failed to get metrics for `azure` leases: %v", err) - } - metrics2, err := m.lClient.Metrics("azure-2-quota-slice") - if err != nil { - return "", fmt.Errorf("failed to get metrics for `azure-2` leases: %v", err) - } - if metrics2.Free > metrics1.Free { - job.UseSecondaryAccount = true - } - case "gcp": - metrics1, err := m.lClient.Metrics("gcp-quota-slice") - if err != nil { - return "", fmt.Errorf("failed to get metrics for `gcp` leases: %v", err) - } - metrics2, err := m.lClient.Metrics("gcp-openshift-gce-devel-ci-2-quota-slice") - if err != nil { - return "", fmt.Errorf("failed to get metrics for `gcp-openshift-gce-devel-ci-2` leases: %v", err) - } - if metrics2.Free > metrics1.Free { - job.UseSecondaryAccount = true - } - } + // Delegate account dispersement to Test Platform via the platform's + // cluster-profile set, which randomly selects an underlying account at + // runtime. Non-amd64 launches keep the default per-platform profile. + if req.Architecture == "amd64" { + job.CloudProfileSet = platformProfileSets[req.Platform] } msg, err := func() (string, error) { diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index a3a5a1c31..aebb1050b 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -5,6 +5,27 @@ import ( "testing" ) +func Test_platformProfileSets(t *testing.T) { + tests := []struct { + name string + platform string + want string + }{ + {name: "aws maps to its profile set", platform: "aws", want: "openshift-org-aws"}, + {name: "azure maps to its profile set", platform: "azure", want: "openshift-org-azure"}, + {name: "gcp maps to its profile set", platform: "gcp", want: "openshift-org-gcp"}, + {name: "unknown platform has no profile set", platform: "metal", want: ""}, + {name: "empty platform has no profile set", platform: "", want: ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := platformProfileSets[tt.platform]; got != tt.want { + t.Errorf("platformProfileSets[%q] = %q, want %q", tt.platform, got, tt.want) + } + }) + } +} + func Test_containsValidVersion(t *testing.T) { type args struct { listOfImageOrVersionOrPRs []string diff --git a/pkg/manager/prow.go b/pkg/manager/prow.go index d2a3e5f8d..a92c25f32 100644 --- a/pkg/manager/prow.go +++ b/pkg/manager/prow.go @@ -603,21 +603,11 @@ func (m *jobManager) newJob(job *Job) (string, error) { } } - // if a step based config, launch should now be the test config we will run; time to update the config for lease balancing - if job.UseSecondaryAccount { - switch job.Platform { - case "aws": - if err := convertAWSToAWS2(pj, sourceConfig); err != nil { - return "", fmt.Errorf("failed updating aws job to aws-2: %w", err) - } - case "gcp": - if err := convertGCPToGCP2(pj, sourceConfig); err != nil { - return "", fmt.Errorf("failed updating gcp job to gcp-openshift-gce-devel-ci-2: %w", err) - } - case "azure": - if err := convertAzureToAzure2(pj, sourceConfig); err != nil { - return "", fmt.Errorf("failed updating azure job to azure-2: %w", err) - } + // if a cluster-profile set was selected, apply it so Test Platform picks + // and balances the underlying account at runtime + if job.CloudProfileSet != "" { + if err := applyClusterProfile(pj, sourceConfig, job.CloudProfileSet); err != nil { + return "", fmt.Errorf("failed applying cluster profile %q: %w", job.CloudProfileSet, err) } } @@ -1822,18 +1812,13 @@ func (e *resolvedEnvironment) Lookup(name string) string { return "" } -func convertToAccount2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration, profileName, profileSecret, accountDomain string) error { +// applyClusterProfile points the job's `launch` test at the given cluster +// profile (typically a profile set such as "openshift-org-gcp"). Only the +// cloud-cluster-profile label and the launch test's ClusterProfile are set; +// the per-account secret volume and BASE_DOMAIN are intentionally left alone, +// as the runtime resolves those from the account the profile set selects. +func applyClusterProfile(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration, profileName string) error { job.Labels["ci-operator.openshift.io/cloud-cluster-profile"] = profileName - for index, volume := range job.Spec.PodSpec.Volumes { - // TODO: only some ci-chat-bot jobs have this; check if they can all be removed - if volume.Name == "cluster-profile" { - if volume.Projected == nil { - volume.Projected = &corev1.ProjectedVolumeSource{} - } - volume.Projected.Sources = []corev1.VolumeProjection{{Secret: &corev1.SecretProjection{LocalObjectReference: corev1.LocalObjectReference{Name: profileSecret}}}} - job.Spec.PodSpec.Volumes[index] = volume - } - } var matchedTarget *citools.TestStepConfiguration for _, test := range sourceConfig.Tests { if test.As == "launch" { @@ -1848,20 +1833,5 @@ func convertToAccount2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuil return fmt.Errorf("invalid job; `launch` test is not a multistage test") } matchedTarget.MultiStageTestConfiguration.ClusterProfile = citools.ClusterProfile(profileName) - if accountDomain != "" && matchedTarget.MultiStageTestConfiguration != nil && matchedTarget.MultiStageTestConfiguration.Environment != nil { - matchedTarget.MultiStageTestConfiguration.Environment["BASE_DOMAIN"] = accountDomain - } return nil } - -func convertAWSToAWS2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration) error { - return convertToAccount2(job, sourceConfig, "aws-2", "cluster-secrets-aws-2", "aws-2.ci.openshift.org") -} - -func convertAzureToAzure2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration) error { - return convertToAccount2(job, sourceConfig, "azure-2", "cluster-secrets-azure-2", "ci2.azure.devcluster.openshift.com") -} - -func convertGCPToGCP2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration) error { - return convertToAccount2(job, sourceConfig, "gcp-openshift-gce-devel-ci-2", "cluster-secrets-gcp-openshift-gce-devel-ci-2", "") -} diff --git a/pkg/manager/prow_test.go b/pkg/manager/prow_test.go index 83e11d92c..2f790c3a7 100644 --- a/pkg/manager/prow_test.go +++ b/pkg/manager/prow_test.go @@ -5,6 +5,7 @@ import ( "github.com/google/go-cmp/cmp" citools "github.com/openshift/ci-tools/pkg/api" + corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" prowapiv1 "sigs.k8s.io/prow/pkg/apis/prowjobs/v1" ) @@ -280,3 +281,80 @@ func Test_processOperatorPR(t *testing.T) { }) } } + +func Test_applyClusterProfile(t *testing.T) { + newJob := func() *prowapiv1.ProwJob { + return &prowapiv1.ProwJob{ + ObjectMeta: v1.ObjectMeta{ + Labels: map[string]string{"ci-operator.openshift.io/cloud-cluster-profile": "gcp"}, + }, + Spec: prowapiv1.ProwJobSpec{ + PodSpec: &corev1.PodSpec{ + Volumes: []corev1.Volume{{ + Name: "cluster-profile", + VolumeSource: corev1.VolumeSource{ + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{{ + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{Name: "cluster-secrets-gcp"}, + }, + }}, + }, + }, + }}, + }, + }, + } + } + newConfig := func() *citools.ReleaseBuildConfiguration { + return &citools.ReleaseBuildConfiguration{ + Tests: []citools.TestStepConfiguration{{ + As: "launch", + MultiStageTestConfiguration: &citools.MultiStageTestConfiguration{ + ClusterProfile: "gcp", + Environment: citools.TestEnvironment{"BASE_DOMAIN": "gcp.devcluster.openshift.com"}, + }, + }}, + } + } + + t.Run("sets label and launch ClusterProfile without touching secret volume or BASE_DOMAIN", func(t *testing.T) { + job := newJob() + config := newConfig() + if err := applyClusterProfile(job, config, "openshift-org-gcp"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := job.Labels["ci-operator.openshift.io/cloud-cluster-profile"]; got != "openshift-org-gcp" { + t.Errorf("label = %q, want %q", got, "openshift-org-gcp") + } + if got := config.Tests[0].MultiStageTestConfiguration.ClusterProfile; got != "openshift-org-gcp" { + t.Errorf("launch ClusterProfile = %q, want %q", got, "openshift-org-gcp") + } + // the per-account secret volume must be left untouched; the runtime + // resolves the secret from the account the profile set selects + gotSecret := job.Spec.PodSpec.Volumes[0].Projected.Sources[0].Secret.Name + if gotSecret != "cluster-secrets-gcp" { + t.Errorf("cluster-profile secret = %q, want it left as %q", gotSecret, "cluster-secrets-gcp") + } + // BASE_DOMAIN must be left untouched + if got := config.Tests[0].MultiStageTestConfiguration.Environment["BASE_DOMAIN"]; got != "gcp.devcluster.openshift.com" { + t.Errorf("BASE_DOMAIN = %q, want it left unchanged", got) + } + }) + + t.Run("errors when no launch test is present", func(t *testing.T) { + job := newJob() + config := &citools.ReleaseBuildConfiguration{Tests: []citools.TestStepConfiguration{{As: "other"}}} + if err := applyClusterProfile(job, config, "openshift-org-gcp"); err == nil { + t.Fatal("expected error for missing launch test, got nil") + } + }) + + t.Run("errors when launch test is not multistage", func(t *testing.T) { + job := newJob() + config := &citools.ReleaseBuildConfiguration{Tests: []citools.TestStepConfiguration{{As: "launch"}}} + if err := applyClusterProfile(job, config, "openshift-org-gcp"); err == nil { + t.Fatal("expected error for non-multistage launch test, got nil") + } + }) +} diff --git a/pkg/manager/types.go b/pkg/manager/types.go index a8dcaed4a..3d921beb5 100644 --- a/pkg/manager/types.go +++ b/pkg/manager/types.go @@ -486,7 +486,10 @@ type Job struct { WorkflowName string - UseSecondaryAccount bool + // CloudProfileSet is the cluster-profile set (e.g. "openshift-org-gcp") + // to launch under, delegating account selection to Test Platform. Empty + // means use the default per-platform profile. + CloudProfileSet string Operator OperatorInfo CatalogComplete bool