diff --git a/cmd/branchingconfigmanagers/frequency-reducer/main.go b/cmd/branchingconfigmanagers/frequency-reducer/main.go index 8cbca358818..bfd60265934 100644 --- a/cmd/branchingconfigmanagers/frequency-reducer/main.go +++ b/cmd/branchingconfigmanagers/frequency-reducer/main.go @@ -79,6 +79,18 @@ func updateIntervalFieldsForMatchedSteps( if err != nil { return } + + pastVersion, err := version.GetPastVersion() + if err != nil { + logrus.Warningf("Can't get past version for %s: %v", version.GetVersion(), err) + pastVersion = "" + } + pastPastVersion, err := version.GetPastPastVersion() + if err != nil { + logrus.Debugf("Can't get past-past version for %s: %v", version.GetVersion(), err) + pastPastVersion = "" + } + if configuration.Info.Metadata.Org == "openshift" || configuration.Info.Metadata.Org == "openshift-priv" { for _, test := range configuration.Configuration.Tests { if !strings.Contains(test.As, "mirror-nightly-image") && !strings.Contains(test.As, "promote-") { @@ -93,7 +105,7 @@ func updateIntervalFieldsForMatchedSteps( if !correctCron { *test.Cron = generateMonthlyCron() } - } else if testVersion.GetVersion() == version.GetPastPastVersion() { + } else if pastPastVersion != "" && testVersion.GetVersion() == pastPastVersion { correctCron, err := isExecutedAtMostXTimesAMonth(*test.Cron, 2) if err != nil { logrus.Warningf("Can't parse cron string %s", *test.Cron) @@ -102,7 +114,7 @@ func updateIntervalFieldsForMatchedSteps( if !correctCron { *test.Cron = generateBiWeeklyCron() } - } else if testVersion.GetVersion() == version.GetPastVersion() { + } else if pastVersion != "" && testVersion.GetVersion() == pastVersion { correctCron, err := isExecutedAtMostXTimesAMonth(*test.Cron, 4) if err != nil { logrus.Warningf("Can't parse cron string %s", *test.Cron) @@ -125,7 +137,7 @@ func updateIntervalFieldsForMatchedSteps( test.Cron = &cronExpr test.Interval = nil } - } else if testVersion.GetVersion() == version.GetPastPastVersion() { + } else if pastPastVersion != "" && testVersion.GetVersion() == pastPastVersion { duration, err := time.ParseDuration(*test.Interval) if err != nil { logrus.Warningf("Can't parse interval string %s", *test.Cron) @@ -136,7 +148,7 @@ func updateIntervalFieldsForMatchedSteps( test.Cron = &cronExpr test.Interval = nil } - } else if testVersion.GetVersion() == version.GetPastVersion() { + } else if pastVersion != "" && testVersion.GetVersion() == pastVersion { duration, err := time.ParseDuration(*test.Interval) if err != nil { logrus.Warningf("Can't parse interval string %s", *test.Cron) diff --git a/cmd/branchingconfigmanagers/tide-config-manager/main.go b/cmd/branchingconfigmanagers/tide-config-manager/main.go index 7e7ac595061..142a7c94c57 100644 --- a/cmd/branchingconfigmanagers/tide-config-manager/main.go +++ b/cmd/branchingconfigmanagers/tide-config-manager/main.go @@ -484,40 +484,21 @@ func (ve *verifiedEvent) ModifyQuery(q *prowconfig.TideQuery, repo string) { q.Labels = sets.List(reqLabels) } -// isVersionedBranch checks if a branch name matches release-4.x or openshift-4.x pattern +// isVersionedBranch checks if a branch name matches release-X.Y or openshift-X.Y pattern +// where X is a major version (4 or higher) and Y is a minor version func isVersionedBranch(branch string) bool { - if strings.HasPrefix(branch, "release-4.") { - versionPart := strings.TrimPrefix(branch, "release-4.") - if isValidMinorVersion(versionPart) { - return true - } - } - - if strings.HasPrefix(branch, "openshift-4.") { - versionPart := strings.TrimPrefix(branch, "openshift-4.") - if isValidMinorVersion(versionPart) { - return true + prefixes := []string{"release-", "openshift-"} + for _, prefix := range prefixes { + if strings.HasPrefix(branch, prefix) { + versionPart := strings.TrimPrefix(branch, prefix) + if api.IsValidOCPVersion(versionPart) { + return true + } } } - return false } -// isValidMinorVersion checks if a string represents a valid minor version (e.g., "9", "10", "15") -func isValidMinorVersion(version string) bool { - if version == "" { - return false - } - - for _, char := range version { - if char < '0' || char > '9' { - return false - } - } - - return version != "0" -} - func (ve *verifiedEvent) GetDataFromProwConfig(*prowconfig.ProwConfig) { } @@ -601,9 +582,13 @@ func reconcile(currentOCPVersion, lifecyclePhase string, config *prowconfig.Prow ) } if lifecyclePhase == GeneralAvailability { + pastVersion, pastErr := currentVersion.GetPastVersion() + if pastErr != nil { + return fmt.Errorf("failed to get past version for %s: %w", currentVersion.GetVersion(), pastErr) + } _, err = shardprowconfig.ShardProwConfig(config, target, newGeneralAvailabilityEvent( - currentVersion.GetPastVersion(), + pastVersion, currentVersion.GetVersion(), currentVersion.GetFutureVersion(), repos), diff --git a/cmd/branchingconfigmanagers/tide-config-manager/main_test.go b/cmd/branchingconfigmanagers/tide-config-manager/main_test.go index 05ddc4849ba..05e6184fba5 100644 --- a/cmd/branchingconfigmanagers/tide-config-manager/main_test.go +++ b/cmd/branchingconfigmanagers/tide-config-manager/main_test.go @@ -610,14 +610,14 @@ func TestIsVersionedBranch(t *testing.T) { expected: true, }, { - name: "invalid release branch with version 0", + name: "valid release-4.0 branch", branch: "release-4.0", - expected: false, + expected: true, }, { - name: "invalid openshift branch with version 0", + name: "valid openshift-4.0 branch", branch: "openshift-4.0", - expected: false, + expected: true, }, { name: "invalid release branch with non-numeric version", @@ -674,6 +674,31 @@ func TestIsVersionedBranch(t *testing.T) { branch: "openshift-4.", expected: false, }, + { + name: "valid release-5.0 branch", + branch: "release-5.0", + expected: true, + }, + { + name: "valid openshift-5.0 branch", + branch: "openshift-5.0", + expected: true, + }, + { + name: "valid release-5.1 branch", + branch: "release-5.1", + expected: true, + }, + { + name: "valid openshift-5.1 branch", + branch: "openshift-5.1", + expected: true, + }, + { + name: "valid release-5.10 branch", + branch: "release-5.10", + expected: true, + }, } for _, tt := range tests { diff --git a/cmd/ci-operator-config-mirror/main.go b/cmd/ci-operator-config-mirror/main.go index b9cafc8611c..f039e1133f9 100644 --- a/cmd/ci-operator-config-mirror/main.go +++ b/cmd/ci-operator-config-mirror/main.go @@ -6,7 +6,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/sirupsen/logrus" @@ -237,7 +236,7 @@ func privateBuildRoot(buildRoot *api.BuildRootImageConfiguration) { func privateBaseImages(baseImages map[string]api.ImageStreamTagReference) { for name, reference := range baseImages { - if reference.Namespace == ocpNamespace && isIntegrationImageStream(reference.Name) { + if reference.Namespace == ocpNamespace && api.IsValidOCPVersion(reference.Name) { reference.Name = fmt.Sprintf("%s-priv", reference.Name) reference.Namespace = privatePromotionNamespace baseImages[name] = reference @@ -265,7 +264,3 @@ func privatePromotionConfiguration(promotion *api.PromotionConfiguration) { func strP(str string) *string { return &str } - -func isIntegrationImageStream(name string) bool { - return strings.HasPrefix(name, "4.") -} diff --git a/cmd/cvp-trigger/main.go b/cmd/cvp-trigger/main.go index 7d3cf0a8598..590a575aa71 100644 --- a/cmd/cvp-trigger/main.go +++ b/cmd/cvp-trigger/main.go @@ -153,8 +153,8 @@ func (o options) validateOptions() error { if o.ocpVersion == "" { return fmt.Errorf("required parameter %s was not provided", ocpVersionOption) } - if !strings.HasPrefix(o.ocpVersion, "4") { - return fmt.Errorf("ocp-version must be 4.x or higher") + if !api.IsValidOCPVersion(o.ocpVersion) { + return fmt.Errorf("ocp-version must be in format X.Y where X >= 4 (e.g., 4.15, 5.0)") } if o.operatorPackageName == "" { diff --git a/cmd/ocp-build-data-enforcer/main.go b/cmd/ocp-build-data-enforcer/main.go index d67c9dd50b3..32aaae16331 100644 --- a/cmd/ocp-build-data-enforcer/main.go +++ b/cmd/ocp-build-data-enforcer/main.go @@ -39,6 +39,7 @@ func gatherOptions() (*options, error) { o := &options{PRCreationOptions: &prcreation.PRCreationOptions{}} o.PRCreationOptions.AddFlags(flag.CommandLine) flag.StringVar(&o.ocpBuildDataRepoDir, "ocp-build-data-repo-dir", "../ocp-build-data", "The directory in which the ocp-build-data repository is") + flag.StringVar(&o.majorMinor.Major, "major", "4", "The major version to target") flag.StringVar(&o.majorMinor.Minor, "minor", "6", "The minor version to target") flag.BoolVar(&o.createPRs, "create-prs", false, "If the tool should create PRs") flag.IntVar(&o.prCreationCeiling, "pr-creation-ceiling", 5, "The maximum number of PRs to upsert") @@ -61,7 +62,6 @@ func main() { if err != nil { logrus.WithError(err).Fatal("Failed to gather options") } - opts.majorMinor.Major = "4" configs, err := ocpbuilddata.LoadImageConfigs(opts.ocpBuildDataRepoDir, opts.majorMinor) if err != nil { diff --git a/cmd/registry-replacer/main.go b/cmd/registry-replacer/main.go index 2dc2463dfef..ceaecd63931 100644 --- a/cmd/registry-replacer/main.go +++ b/cmd/registry-replacer/main.go @@ -65,6 +65,7 @@ func gatherOptions() (*options, error) { flag.Var(o.ensureCorrectPromotionDockerfileIngoredRepos, "ensure-correct-promotion-dockerfile-ignored-repos", "Repos that are being ignored when ensuring the correct promotion dockerfile in org/repo notation. Can be passed multiple times.") flag.IntVar(&o.maxConcurrency, "concurrency", 500, "Maximum number of concurrent in-flight goroutines to handle files.") flag.StringVar(&o.ocpBuildDataRepoDir, "ocp-build-data-repo-dir", "../ocp-build-data", "The directory in which the ocp-build-data repository is") + flag.StringVar(&o.currentRelease.Major, "current-release-major", "4", "The major version of the current release that is getting forwarded to from the master branch") flag.StringVar(&o.currentRelease.Minor, "current-release-minor", "6", "The minor version of the current release that is getting forwarded to from the master branch") flag.BoolVar(&o.pruneUnusedReplacements, "prune-unused-replacements", false, "If replacements that match nothing should get pruned from the config. Note that if --apply-replacements is set to false pruning will not take place.") flag.BoolVar(&o.pruneUnusedBaseImages, "prune-unused-base-images", false, "If base images that match nothing should get pruned from the config") @@ -90,9 +91,11 @@ func gatherOptions() (*options, error) { errs = append(errs, errors.New("--ocp-build-data-repo-dir must be set when --ensure-correct-promotion-dockerfile is set")) } if o.currentRelease.Minor == "" { - errs = append(errs, errors.New("--current-release must be set when --ensure-correct-promotion-dockerfile is set")) + errs = append(errs, errors.New("--current-release-minor must be set when --ensure-correct-promotion-dockerfile is set")) + } + if o.currentRelease.Major == "" { + errs = append(errs, errors.New("--current-release-major must be set when --ensure-correct-promotion-dockerfile is set")) } - o.currentRelease.Major = "4" } return o, utilerrors.NewAggregate(errs) diff --git a/pkg/api/leases.go b/pkg/api/leases.go index 2a1c42e3ef8..6b6db8ac0ce 100644 --- a/pkg/api/leases.go +++ b/pkg/api/leases.go @@ -42,9 +42,10 @@ func IPPoolLeaseForTest(s *MultiStageTestConfigurationLiteral, metadata Metadata } const ( - openshiftBranch = "openshift-4." - releaseBranch = "release-4." - minimumVersion = 16 + // minimumIPPoolMajorVersion and minimumIPPoolMinorVersion define the minimum OCP version + // that supports IP pools (4.16+) + minimumIPPoolMajorVersion = 4 + minimumIPPoolMinorVersion = 16 ) // Currently, we only have the ability to utilize IP pools in 4.16 and later, we want to make sure not to allocate them @@ -53,17 +54,41 @@ func branchValidForIPPoolLease(branch string) bool { if branch == "master" || branch == "main" { return true } - var version string - if strings.HasPrefix(branch, openshiftBranch) { - version = strings.TrimPrefix(branch, openshiftBranch) - } - if strings.HasPrefix(branch, releaseBranch) { - version = strings.TrimPrefix(branch, releaseBranch) - } - number, err := strconv.Atoi(version) - if err != nil { + + major, minor, ok := parseVersionFromBranch(branch) + if !ok { return false } - return number >= minimumVersion + // Version 5.x and later is always valid + if major > minimumIPPoolMajorVersion { + return true + } + // For version 4.x, check if minor version meets minimum requirement + return major == minimumIPPoolMajorVersion && minor >= minimumIPPoolMinorVersion +} + +// parseVersionFromBranch extracts major and minor version from branch names like +// "openshift-4.16", "release-5.0", etc. +func parseVersionFromBranch(branch string) (major, minor int, ok bool) { + prefixes := []string{"openshift-", "release-"} + for _, prefix := range prefixes { + if strings.HasPrefix(branch, prefix) { + versionStr := strings.TrimPrefix(branch, prefix) + parts := strings.Split(versionStr, ".") + if len(parts) != 2 { + continue + } + majorNum, err := strconv.Atoi(parts[0]) + if err != nil { + continue + } + minorNum, err := strconv.Atoi(parts[1]) + if err != nil { + continue + } + return majorNum, minorNum, true + } + } + return 0, 0, false } diff --git a/pkg/api/leases_test.go b/pkg/api/leases_test.go index 9e6fc63608e..7b44a350ca7 100644 --- a/pkg/api/leases_test.go +++ b/pkg/api/leases_test.go @@ -84,6 +84,36 @@ func TestIPPoolLeaseForTest(t *testing.T) { tests: MultiStageTestConfigurationLiteral{ClusterProfile: ClusterProfileAWS}, metadata: Metadata{Branch: "release-4.10"}, }, + { + name: "aws, with 5.0 branch (should be valid)", + tests: MultiStageTestConfigurationLiteral{ClusterProfile: ClusterProfileAWS}, + metadata: Metadata{Branch: "release-5.0"}, + expected: StepLease{ + ResourceType: "aws-ip-pools", + Env: DefaultIPPoolLeaseEnv, + Count: 13, + }, + }, + { + name: "aws, with openshift-5.0 branch (should be valid)", + tests: MultiStageTestConfigurationLiteral{ClusterProfile: ClusterProfileAWS}, + metadata: Metadata{Branch: "openshift-5.0"}, + expected: StepLease{ + ResourceType: "aws-ip-pools", + Env: DefaultIPPoolLeaseEnv, + Count: 13, + }, + }, + { + name: "aws, with 5.1 branch (should be valid)", + tests: MultiStageTestConfigurationLiteral{ClusterProfile: ClusterProfileAWS}, + metadata: Metadata{Branch: "release-5.1"}, + expected: StepLease{ + ResourceType: "aws-ip-pools", + Env: DefaultIPPoolLeaseEnv, + Count: 13, + }, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/api/metadata.go b/pkg/api/metadata.go index daeab83aea9..2ea76c3796d 100644 --- a/pkg/api/metadata.go +++ b/pkg/api/metadata.go @@ -111,14 +111,14 @@ func IsCiopConfigCM(name string) bool { } var releaseBranches = regexp.MustCompile(`^(release|enterprise|openshift)-([1-3])\.[0-9]+(?:\.[0-9]+)?$`) -var fourXBranches = regexp.MustCompile(`^(release|enterprise|openshift)-(4\.[0-9]+)$`) +var fourXAndLaterBranches = regexp.MustCompile(`^(release|enterprise|openshift)-([4-9]\.[0-9]+)$`) func FlavorForBranch(branch string) string { var flavor string if branch == "master" || branch == "main" { flavor = branch - } else if m := fourXBranches.FindStringSubmatch(branch); m != nil { - flavor = m[2] // the 4.x release string + } else if m := fourXAndLaterBranches.FindStringSubmatch(branch); m != nil { + flavor = m[2] // the 4.x, 5.x, etc. release string } else if m := releaseBranches.FindStringSubmatch(branch); m != nil { flavor = m[2] + ".x" } else { diff --git a/pkg/api/metadata_test.go b/pkg/api/metadata_test.go index 7d3f3296454..2a919fcdc22 100644 --- a/pkg/api/metadata_test.go +++ b/pkg/api/metadata_test.go @@ -186,6 +186,21 @@ func TestMetadata_ConfigMapName(t *testing.T) { branch: "release-4.2", expected: "ci-operator-4.2-configs", }, + { + name: "release 5.0 branch goes to 5.0 configmap", + branch: "release-5.0", + expected: "ci-operator-5.0-configs", + }, + { + name: "openshift 5.0 branch goes to 5.0 configmap", + branch: "openshift-5.0", + expected: "ci-operator-5.0-configs", + }, + { + name: "release 5.1 branch goes to 5.1 configmap", + branch: "release-5.1", + expected: "ci-operator-5.1-configs", + }, } for _, testCase := range testCases { t.Run(testCase.expected, func(t *testing.T) { @@ -364,6 +379,26 @@ func TestFlavorForBranch(t *testing.T) { branch: "release-4.2", expected: "4.2", }, + { + name: "release 5.0 branch goes to 5.0 configmap", + branch: "release-5.0", + expected: "5.0", + }, + { + name: "openshift 5.0 branch goes to 5.0 configmap", + branch: "openshift-5.0", + expected: "5.0", + }, + { + name: "release 5.1 branch goes to 5.1 configmap", + branch: "release-5.1", + expected: "5.1", + }, + { + name: "enterprise 5.0 branch goes to 5.0 configmap", + branch: "enterprise-5.0", + expected: "5.0", + }, } for _, testCase := range testCases { t.Run(testCase.expected, func(t *testing.T) { diff --git a/pkg/api/ocplifecycle/ocplifecycle.go b/pkg/api/ocplifecycle/ocplifecycle.go index d84ebe6a5fa..c378ce3aadc 100644 --- a/pkg/api/ocplifecycle/ocplifecycle.go +++ b/pkg/api/ocplifecycle/ocplifecycle.go @@ -13,7 +13,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" - cioperatorapi "github.com/openshift/ci-tools/pkg/api" + "github.com/openshift/ci-tools/pkg/api" ) // LoadConfig loads the lifecycle configuration from a given localtion. @@ -210,12 +210,25 @@ func (m MajorMinor) Less(other MajorMinor) bool { return m.Minor < other.Minor } -func (m MajorMinor) GetPastVersion() string { - return fmt.Sprintf("%d.%d", m.Major, m.Minor-1) +// GetPastVersion returns the previous version using VersionTransitionOverrides. +// Returns an error if no previous version can be determined (e.g., missing override for X.0). +func (m MajorMinor) GetPastVersion() (string, error) { + current := m.GetVersion() + return api.GetPreviousVersionSimple(current) } -func (m MajorMinor) GetPastPastVersion() string { - return fmt.Sprintf("%d.%d", m.Major, m.Minor-1) +// GetPastPastVersion returns the version before the previous version. +// Returns an error if any step in the chain fails. +func (m MajorMinor) GetPastPastVersion() (string, error) { + pastVersion, err := m.GetPastVersion() + if err != nil { + return "", fmt.Errorf("failed to get past version: %w", err) + } + pastMM, err := ParseMajorMinor(pastVersion) + if err != nil { + return "", fmt.Errorf("failed to parse past version %s: %w", pastVersion, err) + } + return pastMM.GetPastVersion() } func (m MajorMinor) GetVersion() string { @@ -251,16 +264,12 @@ func ParseMajorMinor(version string) (*MajorMinor, error) { return &MajorMinor{Major: int(parsedMajor), Minor: int(parsedMinor)}, nil } -// ProvidesSignalForVersion returns a version (expected to be an "." OpenShift version) for which -// the job will provide relevant continuous CI signal that can be consumed by downstream tooling like Sippy. Generally +// ProvidesSignalForVersion returns the OCP version for which a job provides meaningful CI quality signal. Only // jobs that test CI or Nightly payloads provide such relevant quality signal, so this method approximates identifying // these jobs by checking whether the source ci-operator configuration contains a `latest` release that is a candidate // version (which means one of the ci/nightly payloads will be fetched from release-controller). -// -// Jobs that test an ephemeral OCP payload built for a PR or install a named released version (EC/RC/GA) do not provide -// relevant CI signal for a version. -func ProvidesSignalForVersion(configSpec *cioperatorapi.ReleaseBuildConfiguration) string { - if release, found := configSpec.Releases[cioperatorapi.LatestReleaseName]; found && release.Candidate != nil { +func ProvidesSignalForVersion(configSpec *api.ReleaseBuildConfiguration) string { + if release, found := configSpec.Releases[api.LatestReleaseName]; found && release.Candidate != nil { return release.Candidate.Version } return "" diff --git a/pkg/api/version.go b/pkg/api/version.go new file mode 100644 index 00000000000..dda7ad2d2bf --- /dev/null +++ b/pkg/api/version.go @@ -0,0 +1,115 @@ +package api + +import ( + "fmt" + "sort" + "strconv" + "strings" +) + +// VersionTransitionOverrides defines explicit "previous" versions for cross-major transitions. +// First entry is the primary previous. If not in map, natural progression (X.Y-1) is used. +var VersionTransitionOverrides = map[string][]string{ + "5.0": {"4.22"}, + // "5.1": {"5.0", "4.23"}, +} + +type ParsedVersion struct { + Major int + Minor int +} + +func (v ParsedVersion) String() string { + return fmt.Sprintf("%d.%d", v.Major, v.Minor) +} + +func ParseVersion(version string) (ParsedVersion, error) { + parts := strings.Split(version, ".") + if len(parts) != 2 { + return ParsedVersion{}, fmt.Errorf("invalid version format: %s (expected X.Y)", version) + } + + major, err := strconv.Atoi(parts[0]) + if err != nil { + return ParsedVersion{}, fmt.Errorf("invalid major version in %s: %w", version, err) + } + + minor, err := strconv.Atoi(parts[1]) + if err != nil { + return ParsedVersion{}, fmt.Errorf("invalid minor version in %s: %w", version, err) + } + + return ParsedVersion{Major: major, Minor: minor}, nil +} + +// IsValidOCPVersion validates that the version is in format X.Y where X >= 4. +func IsValidOCPVersion(version string) bool { + parsed, err := ParseVersion(version) + if err != nil { + return false + } + return parsed.Major >= 4 +} + +// GetPreviousVersion returns the primary previous version. For X.0 without override, +// it finds the highest (X-1).* from availableVersions. +func GetPreviousVersion(current string, availableVersions []string) (string, error) { + if overrides, ok := VersionTransitionOverrides[current]; ok && len(overrides) > 0 { + return overrides[0], nil + } + + parsed, err := ParseVersion(current) + if err != nil { + return "", err + } + + if parsed.Minor > 0 { + return fmt.Sprintf("%d.%d", parsed.Major, parsed.Minor-1), nil + } + + if parsed.Major <= 0 { + return "", fmt.Errorf("cannot determine previous version for %s: no previous major version exists", current) + } + + previousMajor := parsed.Major - 1 + var candidates []ParsedVersion + + for _, v := range availableVersions { + pv, err := ParseVersion(v) + if err != nil { + continue + } + if pv.Major == previousMajor { + candidates = append(candidates, pv) + } + } + + if len(candidates) == 0 { + return "", fmt.Errorf("cannot determine previous version for %s: no %d.x versions found in available versions", current, previousMajor) + } + + sort.Slice(candidates, func(i, j int) bool { + return candidates[i].Minor > candidates[j].Minor + }) + + return candidates[0].String(), nil +} + +// GetPreviousVersionSimple returns the primary previous version without availableVersions list. +// For X.0 without override, returns error. +func GetPreviousVersionSimple(current string) (string, error) { + if overrides, ok := VersionTransitionOverrides[current]; ok && len(overrides) > 0 { + return overrides[0], nil + } + + parsed, err := ParseVersion(current) + if err != nil { + return "", err + } + + if parsed.Minor > 0 { + return fmt.Sprintf("%d.%d", parsed.Major, parsed.Minor-1), nil + } + + return "", fmt.Errorf("cannot determine previous version for %s: no override defined", current) +} diff --git a/pkg/api/version_test.go b/pkg/api/version_test.go new file mode 100644 index 00000000000..8301a6733fd --- /dev/null +++ b/pkg/api/version_test.go @@ -0,0 +1,200 @@ +package api + +import ( + "testing" +) + +func TestGetPreviousVersion(t *testing.T) { + // Sample available versions for testing + availableVersions := []string{"4.18", "4.19", "4.20", "4.21", "4.22", "5.0", "5.1", "5.2"} + + testCases := []struct { + name string + version string + availableVersions []string + expected string + expectError bool + }{ + { + name: "5.0 uses override to 4.22", + version: "5.0", + availableVersions: availableVersions, + expected: "4.22", + }, + { + name: "5.1 computes previous as 5.0 (natural progression)", + version: "5.1", + availableVersions: availableVersions, + expected: "5.0", + }, + { + name: "5.2 computes previous as 5.1 (natural progression)", + version: "5.2", + availableVersions: availableVersions, + expected: "5.1", + }, + { + name: "4.23 computes previous as 4.22 (natural progression)", + version: "4.23", + availableVersions: availableVersions, + expected: "4.22", + }, + { + name: "4.1 computes previous as 4.0 (natural progression)", + version: "4.1", + availableVersions: availableVersions, + expected: "4.0", + }, + { + name: "6.0 finds highest 5.x from available (no override)", + version: "6.0", + availableVersions: availableVersions, + expected: "5.2", // highest 5.x in availableVersions + }, + { + name: "6.0 with different available versions", + version: "6.0", + availableVersions: []string{"5.0", "5.1", "5.5", "4.22"}, + expected: "5.5", // highest 5.x + }, + { + name: "4.0 finds highest 3.x from available", + version: "4.0", + availableVersions: []string{"3.10", "3.11", "3.9"}, + expected: "3.11", // highest 3.x + }, + { + name: "4.0 with no 3.x available", + version: "4.0", + availableVersions: []string{"4.1", "4.2"}, + expectError: true, // no 3.x versions available + }, + { + name: "invalid version format", + version: "invalid", + availableVersions: availableVersions, + expectError: true, + }, + { + name: "version with three parts", + version: "4.22.1", + availableVersions: availableVersions, + expectError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := GetPreviousVersion(tc.version, tc.availableVersions) + if tc.expectError { + if err == nil { + t.Errorf("expected error for version %s, got result %s", tc.version, result) + } + } else { + if err != nil { + t.Errorf("unexpected error for version %s: %v", tc.version, err) + } + if result != tc.expected { + t.Errorf("GetPreviousVersion(%s) = %s, expected %s", tc.version, result, tc.expected) + } + } + }) + } +} + +func TestParseVersion(t *testing.T) { + testCases := []struct { + input string + expected ParsedVersion + expectError bool + }{ + {"4.22", ParsedVersion{Major: 4, Minor: 22}, false}, + {"5.0", ParsedVersion{Major: 5, Minor: 0}, false}, + {"10.15", ParsedVersion{Major: 10, Minor: 15}, false}, + {"invalid", ParsedVersion{}, true}, + {"4.22.1", ParsedVersion{}, true}, + {"4", ParsedVersion{}, true}, + {"", ParsedVersion{}, true}, + } + + for _, tc := range testCases { + t.Run(tc.input, func(t *testing.T) { + result, err := ParseVersion(tc.input) + if tc.expectError { + if err == nil { + t.Errorf("expected error for %s", tc.input) + } + } else { + if err != nil { + t.Errorf("unexpected error for %s: %v", tc.input, err) + } + if result != tc.expected { + t.Errorf("ParseVersion(%s) = %v, expected %v", tc.input, result, tc.expected) + } + } + }) + } +} + +func TestGetPreviousVersionSimple(t *testing.T) { + testCases := []struct { + name string + version string + expected string + expectError bool + }{ + { + name: "5.0 uses override to 4.22", + version: "5.0", + expected: "4.22", + }, + { + name: "5.1 computes previous as 5.0 (natural progression)", + version: "5.1", + expected: "5.0", + }, + { + name: "5.2 computes previous as 5.1 (natural progression)", + version: "5.2", + expected: "5.1", + }, + { + name: "4.22 computes previous as 4.21 (natural progression)", + version: "4.22", + expected: "4.21", + }, + { + name: "4.1 computes previous as 4.0 (natural progression)", + version: "4.1", + expected: "4.0", + }, + { + name: "6.0 fails without override (cannot determine previous major's last minor)", + version: "6.0", + expectError: true, + }, + { + name: "invalid version format", + version: "invalid", + expectError: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result, err := GetPreviousVersionSimple(tc.version) + if tc.expectError { + if err == nil { + t.Errorf("expected error for version %s, got result %s", tc.version, result) + } + } else { + if err != nil { + t.Errorf("unexpected error for version %s: %v", tc.version, err) + } + if result != tc.expected { + t.Errorf("GetPreviousVersionSimple(%s) = %s, expected %s", tc.version, result, tc.expected) + } + } + }) + } +} diff --git a/pkg/api/zz_generated.deepcopy.go b/pkg/api/zz_generated.deepcopy.go index 7d7d4c9e95b..954735e42b2 100644 --- a/pkg/api/zz_generated.deepcopy.go +++ b/pkg/api/zz_generated.deepcopy.go @@ -1308,6 +1308,21 @@ func (in *OutputImageTagStepConfiguration) DeepCopy() *OutputImageTagStepConfigu return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ParsedVersion) DeepCopyInto(out *ParsedVersion) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ParsedVersion. +func (in *ParsedVersion) DeepCopy() *ParsedVersion { + if in == nil { + return nil + } + out := new(ParsedVersion) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineImageCacheStepConfiguration) DeepCopyInto(out *PipelineImageCacheStepConfiguration) { *out = *in diff --git a/pkg/jobrunaggregator/jobrunaggregatoranalyzer/pass_fail.go b/pkg/jobrunaggregator/jobrunaggregatoranalyzer/pass_fail.go index c7c607b3eae..a0c8d4728ac 100644 --- a/pkg/jobrunaggregator/jobrunaggregatoranalyzer/pass_fail.go +++ b/pkg/jobrunaggregator/jobrunaggregatoranalyzer/pass_fail.go @@ -3,7 +3,6 @@ package jobrunaggregatoranalyzer import ( "context" "fmt" - "strconv" "strings" "sync" "time" @@ -12,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" + "github.com/openshift/ci-tools/pkg/api" "github.com/openshift/ci-tools/pkg/jobrunaggregator/jobrunaggregatorapi" "github.com/openshift/ci-tools/pkg/jobrunaggregator/jobrunaggregatorlib" "github.com/openshift/ci-tools/pkg/junit" @@ -208,27 +208,6 @@ func (a *weeklyAverageFromTenDays) getAggregatedTestRuns(ctx context.Context) (m return a.aggregatedTestRunsByName, a.queryTestRunsErr } -func getMajor(in string) (int, error) { - major, err := strconv.ParseInt(strings.Split(in, ".")[0], 10, 32) - if err != nil { - return 0, err - } - return int(major), err -} - -func getMinor(in string) (int, error) { - parts := strings.Split(in, ".") - if len(parts) >= 2 { - minor, err := strconv.ParseInt(strings.Split(in, ".")[1], 10, 32) - if err != nil { - return 0, err - } - return int(minor), err - } - - return 0, fmt.Errorf("unable to get minor from version %s", in) -} - func normalizeJobName(jobName, fromRelease, toRelease string) string { newJobName := strings.Replace(jobName, toRelease, "", -1) return strings.Replace(newJobName, fromRelease, "", -1) @@ -239,7 +218,7 @@ func (a *weeklyAverageFromTenDays) getNormalizedFallBackJobName(ctx context.Cont if err != nil { return jobName, err } - var targetFromRelease, targetToRelease string + var job *jobrunaggregatorapi.JobRowWithVariants for _, j := range allJobs { if j.JobName == jobName { @@ -247,40 +226,46 @@ func (a *weeklyAverageFromTenDays) getNormalizedFallBackJobName(ctx context.Cont break } } - if job != nil { - if len(job.FromRelease.StringVal) > 0 { - fromReleaseMajor, err1 := getMajor(job.FromRelease.StringVal) - fromReleaseMinor, err2 := getMinor(job.FromRelease.StringVal) - if err1 != nil || err2 != nil { - fmt.Printf("Error parsing from release %s. Will not fall back to previous release data.\n", job.FromRelease) - return jobName, nil - } - targetFromRelease = fmt.Sprintf("%d.%d", fromReleaseMajor, fromReleaseMinor-1) + + if job == nil { + return jobName, nil + } + + var targetFromRelease, targetToRelease string + + if len(job.FromRelease.StringVal) > 0 { + prev, err := api.GetPreviousVersionSimple(job.FromRelease.StringVal) + if err != nil { + fmt.Printf("Error determining previous version for from release %s: %v. Will not fall back to previous release data.\n", job.FromRelease, err) + return jobName, nil } - if len(job.Release) > 0 { - toReleaseMajor, err1 := getMajor(job.Release) - toReleaseMinor, err2 := getMinor(job.Release) - if err1 != nil || err2 != nil { - fmt.Printf("Error parsing release %s. Will not fall back to previous release data.\n", job.Release) - return jobName, nil - } - targetToRelease = fmt.Sprintf("%d.%d", toReleaseMajor, toReleaseMinor-1) + targetFromRelease = prev + } + + if len(job.Release) > 0 { + prev, err := api.GetPreviousVersionSimple(job.Release) + if err != nil { + fmt.Printf("Error determining previous version for release %s: %v. Will not fall back to previous release data.\n", job.Release, err) + return jobName, nil } + targetToRelease = prev + } - normalizedJobName := normalizeJobName(job.JobName, job.FromRelease.StringVal, job.Release) - for _, j := range allJobs { - if j.Architecture == job.Architecture && - j.Topology == job.Topology && - j.Network == job.Network && - j.Platform == job.Platform && - j.FromRelease.StringVal == targetFromRelease && - j.Release == targetToRelease && - j.IPMode == job.IPMode && - normalizeJobName(j.JobName, j.FromRelease.StringVal, j.Release) == normalizedJobName { - return j.JobName, nil - } + normalizedJobName := normalizeJobName(job.JobName, job.FromRelease.StringVal, job.Release) + + for _, j := range allJobs { + if j.Architecture == job.Architecture && + j.Topology == job.Topology && + j.Network == job.Network && + j.Platform == job.Platform && + j.FromRelease.StringVal == targetFromRelease && + j.Release == targetToRelease && + j.IPMode == job.IPMode && + normalizeJobName(j.JobName, j.FromRelease.StringVal, j.Release) == normalizedJobName { + return j.JobName, nil } } + return jobName, nil } diff --git a/pkg/jobrunaggregator/jobrunhistoricaldataanalyzer/util.go b/pkg/jobrunaggregator/jobrunhistoricaldataanalyzer/util.go index 0e7cbe2d31e..2a150289597 100644 --- a/pkg/jobrunaggregator/jobrunhistoricaldataanalyzer/util.go +++ b/pkg/jobrunaggregator/jobrunhistoricaldataanalyzer/util.go @@ -9,9 +9,9 @@ import ( "os" "sort" "strconv" - "strings" "time" + "github.com/openshift/ci-tools/pkg/api" "github.com/openshift/ci-tools/pkg/jobrunaggregator/jobrunaggregatorapi" ) @@ -97,24 +97,37 @@ func fetchCurrentRelease() (current string, previous string, err error) { if err := json.Unmarshal(data, &sippyRelease); err != nil { return "", "", err } - sorted := []int{} + + var validVersions []string + var parsedVersions []api.ParsedVersion + for _, d := range sippyRelease.Releases { - if !strings.Contains(d, "4.") { + pv, err := api.ParseVersion(d) + if err != nil || pv.Major < 4 { continue } - minorVersionString := strings.TrimPrefix(d, "4.") - minorVersion, err := strconv.Atoi(minorVersionString) - if err != nil { - continue - } - sorted = append(sorted, minorVersion) + validVersions = append(validVersions, d) + parsedVersions = append(parsedVersions, pv) + } + + if len(parsedVersions) < 1 { + return "", "", fmt.Errorf("no releases found") } - sort.SliceStable(sorted, func(i, j int) bool { - return sorted[i] > sorted[j] + + sort.SliceStable(parsedVersions, func(i, j int) bool { + if parsedVersions[i].Major != parsedVersions[j].Major { + return parsedVersions[i].Major > parsedVersions[j].Major + } + return parsedVersions[i].Minor > parsedVersions[j].Minor }) - current = fmt.Sprintf("4.%d", sorted[0]) - previous = fmt.Sprintf("4.%d", sorted[1]) - return + + current = parsedVersions[0].String() + previous, err = api.GetPreviousVersion(current, validVersions) + if err != nil { + return "", "", fmt.Errorf("failed to determine previous version for %s: %w", current, err) + } + + return current, previous, nil } func formatTableOutput(data []parsedJobData, filter bool) string { diff --git a/pkg/rehearse/jobs.go b/pkg/rehearse/jobs.go index 3fd3399a720..367e402e14d 100644 --- a/pkg/rehearse/jobs.go +++ b/pkg/rehearse/jobs.go @@ -53,19 +53,28 @@ const ( pjRehearse = "pj-rehearse" ) -// Number of openshift versions -var numVersion = 50 +// Number of minor versions per major version +var numMinorVersions = 50 + +// Supported major versions (4 through 9) +const ( + minMajorVersion = 4 + maxMajorVersion = 9 +) -// Global map that contains relevance of known branches var relevancy = map[string]int{ - "master": numVersion + 1, - "main": numVersion + 1, + "master": 10000, + "main": 10000, } func init() { - for i := 1; i < numVersion; i++ { - relevancy[fmt.Sprintf("release-4.%d", i)] = i - relevancy[fmt.Sprintf("openshift-4.%d", i)] = i + for major := minMajorVersion; major <= maxMajorVersion; major++ { + baseRelevancy := (major - minMajorVersion) * 100 + for minor := 0; minor < numMinorVersions; minor++ { + rel := baseRelevancy + minor + relevancy[fmt.Sprintf("release-%d.%d", major, minor)] = rel + relevancy[fmt.Sprintf("openshift-%d.%d", major, minor)] = rel + } } } diff --git a/pkg/release/prerelease/client.go b/pkg/release/prerelease/client.go index 0ccc05a6f27..1a59290ec02 100644 --- a/pkg/release/prerelease/client.go +++ b/pkg/release/prerelease/client.go @@ -2,6 +2,7 @@ package prerelease import ( "fmt" + "strings" "github.com/blang/semver" @@ -10,15 +11,27 @@ import ( "github.com/openshift/ci-tools/pkg/release/candidate" ) -// endpoint determines the API endpoint to use for a prerelease func endpoint(prerelease api.Prerelease) string { stream := prerelease.VersionBounds.Stream if stream == "" { - stream = "4-stable" + stream = deriveStreamFromBounds(prerelease.VersionBounds) } return candidate.Endpoint(prerelease.ReleaseDescriptor, "", stream, "/latest") } +func deriveStreamFromBounds(bounds api.VersionBounds) string { + for _, version := range []string{bounds.Lower, bounds.Upper} { + if version == "" { + continue + } + parts := strings.Split(version, ".") + if len(parts) >= 1 && parts[0] != "" { + return fmt.Sprintf("%s-stable", parts[0]) + } + } + return "4-stable" +} + func defaultFields(prerelease api.Prerelease) api.Prerelease { if prerelease.Architecture == "" { prerelease.Architecture = api.ReleaseArchitectureAMD64 @@ -35,22 +48,24 @@ func resolvePullSpec(client release.HTTPClient, endpoint string, bounds api.Vers return candidate.ResolvePullSpecCommon(client, endpoint, &bounds, relative) } -func stable4Latest(client release.HTTPClient) (string, error) { - endpoint := endpoint(api.Prerelease{ReleaseDescriptor: api.ReleaseDescriptor{Product: api.ReleaseProductOCP, Architecture: api.ReleaseArchitectureAMD64}}) - rel, err := candidate.ResolveReleaseCommon(client, endpoint, nil, 0, true) +func stableLatest(client release.HTTPClient, stream string) (string, error) { + ep := candidate.Endpoint( + api.ReleaseDescriptor{Product: api.ReleaseProductOCP, Architecture: api.ReleaseArchitectureAMD64}, + "", stream, "/latest", + ) + rel, err := candidate.ResolveReleaseCommon(client, ep, nil, 0, true) return rel.Name, err } -// Stable4LatestMajorMinor returns the release name for stable-4 stream without the patch in it -// E.g, return 4.15 if the stable latest is 4.15.1 -func Stable4LatestMajorMinor(client release.HTTPClient) (string, error) { - version, err := stable4Latest(client) +// StableLatestMajorMinor returns the latest major.minor from the specified stable stream. +func StableLatestMajorMinor(client release.HTTPClient, stream string) (string, error) { + version, err := stableLatest(client, stream) if err != nil { return "", err } sv, err := semver.Make(version) if err != nil { - return "", fmt.Errorf("failed to make sematic version from %s: %w", version, err) + return "", fmt.Errorf("failed to make semantic version from %s: %w", version, err) } return fmt.Sprintf("%d.%d", sv.Major, sv.Minor), nil } diff --git a/pkg/steps/release/promote.go b/pkg/steps/release/promote.go index 69633f680e3..ca1e5aed7c5 100644 --- a/pkg/steps/release/promote.go +++ b/pkg/steps/release/promote.go @@ -105,10 +105,10 @@ func (s *promotionStep) run(ctx context.Context) error { logger.WithError(err).Warn("Failed to ensure namespaces to promote to in central registry.") } - version, err := prerelease.Stable4LatestMajorMinor(&http.Client{}) + version, err := getLatestStableCLIVersion(&http.Client{}) if err != nil { - logrus.WithError(err).Warn("Failed to determine the sable release version, using 4.14 instead") - version = "4.14" + logrus.WithError(err).Warn("Failed to determine the stable release version, using 4.20 instead") + version = "4.20" } if _, err := steps.RunPod(ctx, s.client, getPromotionPod(imageMirrorTarget, timeStr, s.jobSpec.Namespace(), s.name, version, s.nodeArchitectures), false); err != nil { @@ -222,6 +222,16 @@ func getPublicImageReference(dockerImageReference, publicDockerImageRepository s return strings.Replace(dockerImageReference, splits[0], publicHost, 1) } +func getLatestStableCLIVersion(client *http.Client) (string, error) { + for major := 9; major >= 4; major-- { + stream := fmt.Sprintf("%d-stable", major) + if version, err := prerelease.StableLatestMajorMinor(client, stream); err == nil { + return version, nil + } + } + return "", fmt.Errorf("no stable CLI version found in any stream (tried 9-stable through 4-stable)") +} + func getMirrorCommand(registryConfig string, images []string, loglevel int) string { return fmt.Sprintf("oc image mirror --loglevel=%d --keep-manifest-list --registry-config=%s --max-per-registry=10 %s", loglevel, registryConfig, strings.Join(images, " "))