From bb12570905200e8173879f100b42d967af9762b5 Mon Sep 17 00:00:00 2001 From: thiagoalessio Date: Tue, 17 Mar 2026 12:07:16 +0100 Subject: [PATCH 1/6] OCPCRT-450: Rename convertToAccount2 to applyClusterProfile Renaming this function to better reflect its purpose, especially if at some point we have more than two accounts. Co-Authored-By: Claude Opus 4.6 --- pkg/manager/prow.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/manager/prow.go b/pkg/manager/prow.go index d2a3e5f8d..bfe049709 100644 --- a/pkg/manager/prow.go +++ b/pkg/manager/prow.go @@ -1822,7 +1822,7 @@ func (e *resolvedEnvironment) Lookup(name string) string { return "" } -func convertToAccount2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration, profileName, profileSecret, accountDomain string) error { +func applyClusterProfile(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration, profileName, profileSecret, accountDomain 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 @@ -1855,13 +1855,13 @@ func convertToAccount2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuil } func convertAWSToAWS2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration) error { - return convertToAccount2(job, sourceConfig, "aws-2", "cluster-secrets-aws-2", "aws-2.ci.openshift.org") + return applyClusterProfile(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") + return applyClusterProfile(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", "") + return applyClusterProfile(job, sourceConfig, "gcp-openshift-gce-devel-ci-2", "cluster-secrets-gcp-openshift-gce-devel-ci-2", "") } From c419990c2a1efbe97f57afbdc60be0c5a8b10f09 Mon Sep 17 00:00:00 2001 From: thiagoalessio Date: Tue, 17 Mar 2026 12:16:07 +0100 Subject: [PATCH 2/6] OCPCRT-450: Add CloudAccountProfile struct and platformQuotaSlices map Introduce the data structures that will replace the hardcoded switch block for quota-slice selection. Co-Authored-By: Claude Opus 4.6 --- pkg/manager/manager.go | 32 ++++++++++++++++++++++++++++++++ pkg/manager/types.go | 9 +++++++++ 2 files changed, 41 insertions(+) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index b4b467fbd..02c732e67 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -108,6 +108,38 @@ var HypershiftSupportedVersions = HypershiftSupportedVersionsType{} var reBranchVersion = regexp.MustCompile(`^(openshift-|release-)(\d+\.\d+)$`) var reMajorMinorVersion = regexp.MustCompile(`^(\d+)\.(\d+)$`) +// platformQuotaSlices maps each cloud platform to its available quota-slice +// accounts. The first entry is the primary (default) account. Subsequent entries +// are alternates that can be selected when they have more free resources. +var platformQuotaSlices = map[string][]CloudAccountProfile{ + "aws": { + {QuotaSlice: "aws-quota-slice"}, + { + QuotaSlice: "aws-2-quota-slice", + ProfileName: "aws-2", + ProfileSecret: "cluster-secrets-aws-2", + AccountDomain: "aws-2.ci.openshift.org", + }, + }, + "azure": { + {QuotaSlice: "azure4-quota-slice"}, + { + QuotaSlice: "azure-2-quota-slice", + ProfileName: "azure-2", + ProfileSecret: "cluster-secrets-azure-2", + AccountDomain: "ci2.azure.devcluster.openshift.com", + }, + }, + "gcp": { + {QuotaSlice: "gcp-quota-slice"}, + { + QuotaSlice: "gcp-openshift-gce-devel-ci-2-quota-slice", + ProfileName: "gcp-openshift-gce-devel-ci-2", + ProfileSecret: "cluster-secrets-gcp-openshift-gce-devel-ci-2", + }, + }, +} + func (j Job) IsComplete() bool { return j.Complete || len(j.Credentials) > 0 || (len(j.State) > 0 && j.State != prowapiv1.PendingState) } diff --git a/pkg/manager/types.go b/pkg/manager/types.go index a8dcaed4a..ec8a105de 100644 --- a/pkg/manager/types.go +++ b/pkg/manager/types.go @@ -449,6 +449,15 @@ type JobInput struct { Refs []prowapiv1.Refs } +// CloudAccountProfile holds the parameters needed to redirect a ProwJob to an +// alternate cloud account (e.g. aws-2 instead of the default aws account). +type CloudAccountProfile struct { + QuotaSlice string // boskos resource type, e.g. "aws-2-quota-slice" + ProfileName string // cluster profile name, e.g. "aws-2" + ProfileSecret string // k8s secret name, e.g. "cluster-secrets-aws-2" + AccountDomain string // base domain override (optional), e.g. "aws-2.ci.openshift.org" +} + // Job responds to user requests and tracks the state of the launched // jobs. This object must be recreatable from a ProwJob, but the RequestedChannel // field may be empty to indicate the user has already been notified. From f0ade06a11a4427e4407287fc1844c753f5276a9 Mon Sep 17 00:00:00 2001 From: thiagoalessio Date: Tue, 17 Mar 2026 12:21:59 +0100 Subject: [PATCH 3/6] OCPCRT-450: Add selectCloudAccountProfile with tests Extract the quota-slice selection logic into a standalone function that loops over platformQuotaSlices entries, queries Boskos metrics for each, and returns the profile with the most free resources. The old switch block still exists for now. Co-Authored-By: Claude Opus 4.6 --- pkg/manager/manager.go | 27 +++++++++ pkg/manager/manager_test.go | 116 ++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 02c732e67..9ed52a8bd 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -140,6 +140,33 @@ var platformQuotaSlices = map[string][]CloudAccountProfile{ }, } +// selectCloudAccountProfile queries Boskos metrics for each quota-slice +// candidate for the given platform and returns the profile with the most free +// resources. Returns nil if the platform has no configured accounts or if the +// primary (index 0) has the most free resources (no conversion needed). +func selectCloudAccountProfile(platform string, lClient LeaseClient) (*CloudAccountProfile, error) { + accounts, ok := platformQuotaSlices[platform] + if !ok || len(accounts) < 2 { + return nil, nil + } + bestIdx := 0 + bestFree := -1 + for i := range accounts { + metrics, err := lClient.Metrics(accounts[i].QuotaSlice) + if err != nil { + return nil, fmt.Errorf("failed to get metrics for %q leases: %v", accounts[i].QuotaSlice, err) + } + if metrics.Free > bestFree { + bestIdx = i + bestFree = metrics.Free + } + } + if bestIdx == 0 { + return nil, nil + } + return &accounts[bestIdx], nil +} + func (j Job) IsComplete() bool { return j.Complete || len(j.Credentials) > 0 || (len(j.State) > 0 && j.State != prowapiv1.PendingState) } diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index a3a5a1c31..87fce1d9c 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -1,10 +1,126 @@ package manager import ( + "fmt" "strings" "testing" + + "github.com/openshift/ci-tools/pkg/lease" ) +type mockLeaseClient struct { + metrics map[string]lease.Metrics +} + +func (m *mockLeaseClient) Metrics(rtype string) (lease.Metrics, error) { + if metrics, ok := m.metrics[rtype]; ok { + return metrics, nil + } + return lease.Metrics{}, fmt.Errorf("resource type %q not found", rtype) +} + +func Test_selectCloudAccountProfile(t *testing.T) { + tests := []struct { + name string + platform string + metrics map[string]lease.Metrics + wantNil bool + wantProfile string + wantErr bool + }{ + { + name: "platform not in map returns nil", + platform: "metal", + metrics: map[string]lease.Metrics{}, + wantNil: true, + }, + { + name: "primary has more free resources returns nil", + platform: "aws", + metrics: map[string]lease.Metrics{ + "aws-quota-slice": {Free: 10, Leased: 5}, + "aws-2-quota-slice": {Free: 3, Leased: 12}, + }, + wantNil: true, + }, + { + name: "secondary has more free resources returns that profile", + platform: "aws", + metrics: map[string]lease.Metrics{ + "aws-quota-slice": {Free: 2, Leased: 13}, + "aws-2-quota-slice": {Free: 8, Leased: 7}, + }, + wantProfile: "aws-2", + }, + { + name: "equal free counts returns nil (primary wins)", + platform: "aws", + metrics: map[string]lease.Metrics{ + "aws-quota-slice": {Free: 5, Leased: 10}, + "aws-2-quota-slice": {Free: 5, Leased: 10}, + }, + wantNil: true, + }, + { + name: "all zero free returns nil", + platform: "aws", + metrics: map[string]lease.Metrics{ + "aws-quota-slice": {Free: 0, Leased: 15}, + "aws-2-quota-slice": {Free: 0, Leased: 15}, + }, + wantNil: true, + }, + { + name: "gcp secondary wins", + platform: "gcp", + metrics: map[string]lease.Metrics{ + "gcp-quota-slice": {Free: 1, Leased: 14}, + "gcp-openshift-gce-devel-ci-2-quota-slice": {Free: 7, Leased: 8}, + }, + wantProfile: "gcp-openshift-gce-devel-ci-2", + }, + { + name: "azure secondary wins", + platform: "azure", + metrics: map[string]lease.Metrics{ + "azure4-quota-slice": {Free: 3, Leased: 12}, + "azure-2-quota-slice": {Free: 9, Leased: 6}, + }, + wantProfile: "azure-2", + }, + { + name: "metrics error returns error", + platform: "aws", + metrics: map[string]lease.Metrics{}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &mockLeaseClient{metrics: tt.metrics} + got, err := selectCloudAccountProfile(tt.platform, client) + if (err != nil) != tt.wantErr { + t.Errorf("selectCloudAccountProfile() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + return + } + if tt.wantNil && got != nil { + t.Errorf("selectCloudAccountProfile() = %+v, want nil", got) + return + } + if !tt.wantNil && got == nil { + t.Errorf("selectCloudAccountProfile() = nil, want profile %q", tt.wantProfile) + return + } + if !tt.wantNil && got.ProfileName != tt.wantProfile { + t.Errorf("selectCloudAccountProfile() ProfileName = %q, want %q", got.ProfileName, tt.wantProfile) + } + }) + } +} + func Test_containsValidVersion(t *testing.T) { type args struct { listOfImageOrVersionOrPRs []string From 4f481dd047528ae01dd6d2d145ade86c074769cf Mon Sep 17 00:00:00 2001 From: thiagoalessio Date: Tue, 17 Mar 2026 12:25:24 +0100 Subject: [PATCH 4/6] OCPCRT-450: Replace UseSecondaryAccount with CloudAccountProfile selection Got rid of the switch block in favor of the newly introduced `selectCloudAccountProfile`. Also replaced the other switch block with those wrapper functions in `prow.go` with a single nil check that calls applyClusterProfile directly from the profile struct fields. Co-Authored-By: Claude Opus 4.6 --- pkg/manager/manager.go | 41 ++++------------------------------------- pkg/manager/prow.go | 32 +++++--------------------------- pkg/manager/types.go | 2 +- 3 files changed, 10 insertions(+), 65 deletions(-) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 9ed52a8bd..bce876f48 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -2282,44 +2282,11 @@ func (m *jobManager) LaunchJobForUser(req *JobRequest) (string, error) { // 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 - } + profile, err := selectCloudAccountProfile(req.Platform, m.lClient) + if err != nil { + return "", err } + job.CloudAccountProfile = profile } msg, err := func() (string, error) { diff --git a/pkg/manager/prow.go b/pkg/manager/prow.go index bfe049709..8ab276bc2 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 an alternate cloud account was selected for lease balancing, apply it + if job.CloudAccountProfile != nil { + p := job.CloudAccountProfile + if err := applyClusterProfile(pj, sourceConfig, p.ProfileName, p.ProfileSecret, p.AccountDomain); err != nil { + return "", fmt.Errorf("failed applying cluster profile %q: %w", p.ProfileName, err) } } @@ -1853,15 +1843,3 @@ func applyClusterProfile(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBu } return nil } - -func convertAWSToAWS2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration) error { - return applyClusterProfile(job, sourceConfig, "aws-2", "cluster-secrets-aws-2", "aws-2.ci.openshift.org") -} - -func convertAzureToAzure2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration) error { - return applyClusterProfile(job, sourceConfig, "azure-2", "cluster-secrets-azure-2", "ci2.azure.devcluster.openshift.com") -} - -func convertGCPToGCP2(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBuildConfiguration) error { - return applyClusterProfile(job, sourceConfig, "gcp-openshift-gce-devel-ci-2", "cluster-secrets-gcp-openshift-gce-devel-ci-2", "") -} diff --git a/pkg/manager/types.go b/pkg/manager/types.go index ec8a105de..2df0cb3cd 100644 --- a/pkg/manager/types.go +++ b/pkg/manager/types.go @@ -495,7 +495,7 @@ type Job struct { WorkflowName string - UseSecondaryAccount bool + CloudAccountProfile *CloudAccountProfile Operator OperatorInfo CatalogComplete bool From d573e7a4cb923e067242f53d6510b8b5a9b7c579 Mon Sep 17 00:00:00 2001 From: thiagoalessio Date: Tue, 2 Jun 2026 13:33:27 +0200 Subject: [PATCH 5/6] OCPCRT-450: Fall back to default account when Boskos metrics are unavailable Instead of failing the cluster launch when Boskos metrics cannot be retrieved, log a warning and proceed with the default (primary) account. Co-Authored-By: Claude Opus 4.6 --- pkg/manager/manager.go | 8 +++++--- pkg/manager/manager_test.go | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index bce876f48..c8d59b88b 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -142,8 +142,9 @@ var platformQuotaSlices = map[string][]CloudAccountProfile{ // selectCloudAccountProfile queries Boskos metrics for each quota-slice // candidate for the given platform and returns the profile with the most free -// resources. Returns nil if the platform has no configured accounts or if the -// primary (index 0) has the most free resources (no conversion needed). +// resources. Returns nil if the platform has no configured accounts, if the +// primary (index 0) has the most free resources (no conversion needed), or if +// Boskos metrics are unavailable (falls back to the default account). func selectCloudAccountProfile(platform string, lClient LeaseClient) (*CloudAccountProfile, error) { accounts, ok := platformQuotaSlices[platform] if !ok || len(accounts) < 2 { @@ -154,7 +155,8 @@ func selectCloudAccountProfile(platform string, lClient LeaseClient) (*CloudAcco for i := range accounts { metrics, err := lClient.Metrics(accounts[i].QuotaSlice) if err != nil { - return nil, fmt.Errorf("failed to get metrics for %q leases: %v", accounts[i].QuotaSlice, err) + klog.Warningf("Failed to get metrics for %q leases, falling back to default account: %v", accounts[i].QuotaSlice, err) + return nil, nil } if metrics.Free > bestFree { bestIdx = i diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 87fce1d9c..cc31f152d 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -89,10 +89,10 @@ func Test_selectCloudAccountProfile(t *testing.T) { wantProfile: "azure-2", }, { - name: "metrics error returns error", + name: "metrics error falls back to default account", platform: "aws", metrics: map[string]lease.Metrics{}, - wantErr: true, + wantNil: true, }, } for _, tt := range tests { From bf7e82868fd15037416141a7ee7c369e1724403f Mon Sep 17 00:00:00 2001 From: thiagoalessio Date: Tue, 8 Sep 2026 16:31:19 +0200 Subject: [PATCH 6/6] OCPCRT-450: Delegate account dispersement to cluster-profile sets Test Platform superseded the per-account "regular" cluster profiles with cluster-profile sets (openshift-org-{aws,gcp,azure}) that select an underlying account randomly at job runtime, and deleted the gcp-*-quota-slice Boskos resource types (causing "failed to get metrics for gcp leases: 404"). Stop querying Boskos to balance accounts and instead launch amd64 clusters under the platform's profile set, letting Test Platform handle dispersement. - Replace platformQuotaSlices/selectCloudAccountProfile with a static platformProfileSets map. - Replace Job.CloudAccountProfile with Job.CloudProfileSet. - Simplify applyClusterProfile to set only the cloud-cluster-profile label and the launch test's ClusterProfile; leave the per-account secret volume and BASE_DOMAIN alone, as the runtime resolves those from the selected account. - Update tests accordingly. The now-unused LeaseClient plumbing and --lease-server flags are left wired and will be removed in a follow-up. Assisted-by: Claude Opus 4.8 --- pkg/manager/manager.go | 80 +++++------------------- pkg/manager/manager_test.go | 117 ++++-------------------------------- pkg/manager/prow.go | 30 ++++----- pkg/manager/prow_test.go | 78 ++++++++++++++++++++++++ pkg/manager/types.go | 14 ++--- 5 files changed, 118 insertions(+), 201 deletions(-) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index c8d59b88b..84cb12f0f 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -108,65 +108,15 @@ var HypershiftSupportedVersions = HypershiftSupportedVersionsType{} var reBranchVersion = regexp.MustCompile(`^(openshift-|release-)(\d+\.\d+)$`) var reMajorMinorVersion = regexp.MustCompile(`^(\d+)\.(\d+)$`) -// platformQuotaSlices maps each cloud platform to its available quota-slice -// accounts. The first entry is the primary (default) account. Subsequent entries -// are alternates that can be selected when they have more free resources. -var platformQuotaSlices = map[string][]CloudAccountProfile{ - "aws": { - {QuotaSlice: "aws-quota-slice"}, - { - QuotaSlice: "aws-2-quota-slice", - ProfileName: "aws-2", - ProfileSecret: "cluster-secrets-aws-2", - AccountDomain: "aws-2.ci.openshift.org", - }, - }, - "azure": { - {QuotaSlice: "azure4-quota-slice"}, - { - QuotaSlice: "azure-2-quota-slice", - ProfileName: "azure-2", - ProfileSecret: "cluster-secrets-azure-2", - AccountDomain: "ci2.azure.devcluster.openshift.com", - }, - }, - "gcp": { - {QuotaSlice: "gcp-quota-slice"}, - { - QuotaSlice: "gcp-openshift-gce-devel-ci-2-quota-slice", - ProfileName: "gcp-openshift-gce-devel-ci-2", - ProfileSecret: "cluster-secrets-gcp-openshift-gce-devel-ci-2", - }, - }, -} - -// selectCloudAccountProfile queries Boskos metrics for each quota-slice -// candidate for the given platform and returns the profile with the most free -// resources. Returns nil if the platform has no configured accounts, if the -// primary (index 0) has the most free resources (no conversion needed), or if -// Boskos metrics are unavailable (falls back to the default account). -func selectCloudAccountProfile(platform string, lClient LeaseClient) (*CloudAccountProfile, error) { - accounts, ok := platformQuotaSlices[platform] - if !ok || len(accounts) < 2 { - return nil, nil - } - bestIdx := 0 - bestFree := -1 - for i := range accounts { - metrics, err := lClient.Metrics(accounts[i].QuotaSlice) - if err != nil { - klog.Warningf("Failed to get metrics for %q leases, falling back to default account: %v", accounts[i].QuotaSlice, err) - return nil, nil - } - if metrics.Free > bestFree { - bestIdx = i - bestFree = metrics.Free - } - } - if bestIdx == 0 { - return nil, nil - } - return &accounts[bestIdx], nil +// 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 { @@ -2282,13 +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 { - profile, err := selectCloudAccountProfile(req.Platform, m.lClient) - if err != nil { - return "", err - } - job.CloudAccountProfile = profile + // 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 cc31f152d..aebb1050b 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -1,121 +1,26 @@ package manager import ( - "fmt" "strings" "testing" - - "github.com/openshift/ci-tools/pkg/lease" ) -type mockLeaseClient struct { - metrics map[string]lease.Metrics -} - -func (m *mockLeaseClient) Metrics(rtype string) (lease.Metrics, error) { - if metrics, ok := m.metrics[rtype]; ok { - return metrics, nil - } - return lease.Metrics{}, fmt.Errorf("resource type %q not found", rtype) -} - -func Test_selectCloudAccountProfile(t *testing.T) { +func Test_platformProfileSets(t *testing.T) { tests := []struct { - name string - platform string - metrics map[string]lease.Metrics - wantNil bool - wantProfile string - wantErr bool + name string + platform string + want string }{ - { - name: "platform not in map returns nil", - platform: "metal", - metrics: map[string]lease.Metrics{}, - wantNil: true, - }, - { - name: "primary has more free resources returns nil", - platform: "aws", - metrics: map[string]lease.Metrics{ - "aws-quota-slice": {Free: 10, Leased: 5}, - "aws-2-quota-slice": {Free: 3, Leased: 12}, - }, - wantNil: true, - }, - { - name: "secondary has more free resources returns that profile", - platform: "aws", - metrics: map[string]lease.Metrics{ - "aws-quota-slice": {Free: 2, Leased: 13}, - "aws-2-quota-slice": {Free: 8, Leased: 7}, - }, - wantProfile: "aws-2", - }, - { - name: "equal free counts returns nil (primary wins)", - platform: "aws", - metrics: map[string]lease.Metrics{ - "aws-quota-slice": {Free: 5, Leased: 10}, - "aws-2-quota-slice": {Free: 5, Leased: 10}, - }, - wantNil: true, - }, - { - name: "all zero free returns nil", - platform: "aws", - metrics: map[string]lease.Metrics{ - "aws-quota-slice": {Free: 0, Leased: 15}, - "aws-2-quota-slice": {Free: 0, Leased: 15}, - }, - wantNil: true, - }, - { - name: "gcp secondary wins", - platform: "gcp", - metrics: map[string]lease.Metrics{ - "gcp-quota-slice": {Free: 1, Leased: 14}, - "gcp-openshift-gce-devel-ci-2-quota-slice": {Free: 7, Leased: 8}, - }, - wantProfile: "gcp-openshift-gce-devel-ci-2", - }, - { - name: "azure secondary wins", - platform: "azure", - metrics: map[string]lease.Metrics{ - "azure4-quota-slice": {Free: 3, Leased: 12}, - "azure-2-quota-slice": {Free: 9, Leased: 6}, - }, - wantProfile: "azure-2", - }, - { - name: "metrics error falls back to default account", - platform: "aws", - metrics: map[string]lease.Metrics{}, - wantNil: true, - }, + {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) { - client := &mockLeaseClient{metrics: tt.metrics} - got, err := selectCloudAccountProfile(tt.platform, client) - if (err != nil) != tt.wantErr { - t.Errorf("selectCloudAccountProfile() error = %v, wantErr %v", err, tt.wantErr) - return - } - if tt.wantErr { - return - } - if tt.wantNil && got != nil { - t.Errorf("selectCloudAccountProfile() = %+v, want nil", got) - return - } - if !tt.wantNil && got == nil { - t.Errorf("selectCloudAccountProfile() = nil, want profile %q", tt.wantProfile) - return - } - if !tt.wantNil && got.ProfileName != tt.wantProfile { - t.Errorf("selectCloudAccountProfile() ProfileName = %q, want %q", got.ProfileName, tt.wantProfile) + if got := platformProfileSets[tt.platform]; got != tt.want { + t.Errorf("platformProfileSets[%q] = %q, want %q", tt.platform, got, tt.want) } }) } diff --git a/pkg/manager/prow.go b/pkg/manager/prow.go index 8ab276bc2..a92c25f32 100644 --- a/pkg/manager/prow.go +++ b/pkg/manager/prow.go @@ -603,11 +603,11 @@ func (m *jobManager) newJob(job *Job) (string, error) { } } - // if an alternate cloud account was selected for lease balancing, apply it - if job.CloudAccountProfile != nil { - p := job.CloudAccountProfile - if err := applyClusterProfile(pj, sourceConfig, p.ProfileName, p.ProfileSecret, p.AccountDomain); err != nil { - return "", fmt.Errorf("failed applying cluster profile %q: %w", p.ProfileName, 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) } } @@ -1812,18 +1812,13 @@ func (e *resolvedEnvironment) Lookup(name string) string { return "" } -func applyClusterProfile(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" { @@ -1838,8 +1833,5 @@ func applyClusterProfile(job *prowapiv1.ProwJob, sourceConfig *citools.ReleaseBu 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 } 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 2df0cb3cd..3d921beb5 100644 --- a/pkg/manager/types.go +++ b/pkg/manager/types.go @@ -449,15 +449,6 @@ type JobInput struct { Refs []prowapiv1.Refs } -// CloudAccountProfile holds the parameters needed to redirect a ProwJob to an -// alternate cloud account (e.g. aws-2 instead of the default aws account). -type CloudAccountProfile struct { - QuotaSlice string // boskos resource type, e.g. "aws-2-quota-slice" - ProfileName string // cluster profile name, e.g. "aws-2" - ProfileSecret string // k8s secret name, e.g. "cluster-secrets-aws-2" - AccountDomain string // base domain override (optional), e.g. "aws-2.ci.openshift.org" -} - // Job responds to user requests and tracks the state of the launched // jobs. This object must be recreatable from a ProwJob, but the RequestedChannel // field may be empty to indicate the user has already been notified. @@ -495,7 +486,10 @@ type Job struct { WorkflowName string - CloudAccountProfile *CloudAccountProfile + // 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