address review feedback (and more)

- switch from "not-latest" to "latest" flag for hashrelease publish - restructure build to pass validation flags to controllers - create hashrelease.PinnedVersionConfig and version.Data - revert change to version.GitVersion - renames for consistencies - clean unused fns/files
projectcalico · Oct 1, 2024 · 306c8d1 · 306c8d1
1 parent 53c14e2
commit 306c8d1
Show file tree

Hide file tree

Showing 13 changed files with 276 additions and 196 deletions.
diff --git a/release/build/main.go b/release/build/main.go
@@ -23,6 +23,7 @@ import (
 	"gopkg.in/natefinch/lumberjack.v2"
 
 	"github.com/projectcalico/calico/release/internal/config"
+	"github.com/projectcalico/calico/release/internal/hashrelease"
 	"github.com/projectcalico/calico/release/internal/utils"
 	"github.com/projectcalico/calico/release/internal/version"
 	"github.com/projectcalico/calico/release/pkg/controller/branch"
@@ -35,7 +36,7 @@ import (
 )
 
 const (
-	notLatestFlag       = "not-latest"
+	latestFlag          = "latest"
 	skipValidationFlag  = "skip-validation"
 	skipImageScanFlag   = "skip-image-scan"
 	skipBranchCheckFlag = "skip-branch-check"
@@ -149,25 +150,46 @@ func hashreleaseSubCommands(cfg *config.Config) []*cli.Command {
 			},
 			Action: func(c *cli.Context) error {
 				configureLogging("hashrelease-build.log")
-				if !c.Bool(skipValidationFlag) {
-					tasks.PreReleaseValidate(cfg, !c.Bool(skipBranchCheckFlag))
+				if c.Bool(skipValidationFlag) && !c.Bool(skipBranchCheckFlag) {
+					return fmt.Errorf("%s must be set if %s is set", skipBranchCheckFlag, skipValidationFlag)
 				}
 
-				// Create the pinned-version.yaml file and extract the versions and hash.
-				ver, operatorVer, hash := tasks.PinnedVersion(cfg)
-
-				// Check if the hashrelease has already been published.
-				tasks.CheckIfHashReleasePublished(cfg, hash)
-
-				// Build the operator.
-				o := operator.NewController(
+				operatorOpts := []operator.Option{
 					operator.WithRepoRoot(cfg.OperatorConfig.Dir),
 					operator.IsHashRelease(),
-					operator.WithVersion(operatorVer),
 					operator.WithArchitectures(cfg.Arches),
 					operator.WithValidate(!c.Bool(skipValidationFlag)),
-				)
-				if err := o.Build(cfg.TmpFolderPath()); err != nil {
+					operator.WithReleaseBranchValidation(!c.Bool(skipBranchCheckFlag)),
+				}
+				// Clone the operator repository
+				if err := operator.NewController(operatorOpts...).Clone(); err != nil {
+					return err
+				}
+
+				// Create the pinned-version.yaml file and extract the versions and hash.
+				_, data, err := hashrelease.GeneratePinnedVersionFile(hashrelease.PinnedVersionConfig{
+					RootDir:             cfg.RepoRootDir,
+					ReleaseBranchPrefix: cfg.RepoReleaseBranchPrefix,
+					DevTagSuffix:        cfg.DevTagSuffix,
+					Operator:            cfg.OperatorConfig,
+				}, cfg.TmpFolderPath())
+				if err != nil {
+					return err
+				}
+
+				versions := &version.Data{
+					ProductVersion:  version.New(data.ProductVersion),
+					OperatorVersion: version.New(data.Operator.Version),
+				}
+
+				// Check if the hashrelease has already been published.
+				if published := tasks.HashreleasePublished(cfg, data.Hash); published {
+					return fmt.Errorf("hashrelease %s has already been published", data.Hash)
+				}
+
+				// Build the operator
+				operatorOpts = append(operatorOpts, operator.WithVersion(versions.OperatorVersion.FormattedString()))
+				if err := operator.NewController(operatorOpts...).Build(cfg.TmpFolderPath()); err != nil {
 					return err
 				}
 
@@ -176,10 +198,11 @@ func hashreleaseSubCommands(cfg *config.Config) []*cli.Command {
 				opts := []release.Option{
 					release.WithRepoRoot(cfg.RepoRootDir),
 					release.IsHashRelease(),
-					release.WithVersions(ver, operatorVer),
+					release.WithVersions(versions),
 					release.WithOutputDir(dir),
 					release.WithBuildImages(c.Bool(buildImagesFlag)),
-					release.WithPreReleaseValidation(!c.Bool(skipValidationFlag)),
+					release.WithValidate(!c.Bool(skipValidationFlag)),
+					release.WithReleaseBranchValidation(!c.Bool(skipBranchCheckFlag)),
 					release.WithGithubOrg(cfg.Organization),
 					release.WithArchitectures(cfg.Arches),
 				}
@@ -194,7 +217,7 @@ func hashreleaseSubCommands(cfg *config.Config) []*cli.Command {
 
 				// For real releases, release notes are generated prior to building the release. For hash releases,
 				// generate a set of release notes and add them to the hashrelease directory.
-				tasks.ReleaseNotes(cfg, filepath.Join(dir, releaseNotesDir), version.New(ver))
+				tasks.ReleaseNotes(cfg, filepath.Join(dir, releaseNotesDir), versions.ProductVersion)
 
 				// Adjsut the formatting of the generated outputs to match the legacy hashrelease format.
 				return tasks.ReformatHashrelease(cfg, dir)
@@ -206,7 +229,7 @@ func hashreleaseSubCommands(cfg *config.Config) []*cli.Command {
 			Name:  "publish",
 			Usage: "Publish hashrelease from _output/ to hashrelease server",
 			Flags: []cli.Flag{
-				&cli.BoolFlag{Name: notLatestFlag, Usage: "Do not promote this release as the latest for this stream", Value: false},
+				&cli.BoolFlag{Name: latestFlag, Usage: "Promote this release as the latest for this stream", Value: true},
 				&cli.BoolFlag{Name: skipValidationFlag, Usage: "Skip pre-build validation", Value: false},
 				&cli.BoolFlag{Name: skipImageScanFlag, Usage: "Skip sending images to image scan service.", Value: false},
 			},
@@ -233,7 +256,7 @@ func hashreleaseSubCommands(cfg *config.Config) []*cli.Command {
 				if !c.Bool(skipValidationFlag) {
 					tasks.HashreleaseValidate(cfg, c.Bool(skipImageScanFlag))
 				}
-				tasks.HashreleasePush(cfg, dir, !c.Bool(notLatestFlag))
+				tasks.HashreleasePush(cfg, dir, c.Bool(latestFlag))
 				return nil
 			},
 		},
@@ -264,7 +287,7 @@ func releaseSubCommands(cfg *config.Config) []*cli.Command {
 			Usage: "Generate release notes for the next release",
 			Action: func(c *cli.Context) error {
 				configureLogging("release-notes.log")
-				ver, err := version.DetermineReleaseVersion(version.GitVersion(cfg.RepoRootDir), cfg.DevTagSuffix)
+				ver, err := version.DetermineReleaseVersion(version.GitVersion(), cfg.DevTagSuffix)
 				if err != nil {
 					return err
 				}
@@ -285,7 +308,7 @@ func releaseSubCommands(cfg *config.Config) []*cli.Command {
 				configureLogging("release-build.log")
 
 				// Determine the versions to use for the release.
-				ver, err := version.DetermineReleaseVersion(version.GitVersion(cfg.RepoRootDir), cfg.DevTagSuffix)
+				ver, err := version.DetermineReleaseVersion(version.GitVersion(), cfg.DevTagSuffix)
 				if err != nil {
 					return err
 				}
@@ -297,13 +320,16 @@ func releaseSubCommands(cfg *config.Config) []*cli.Command {
 				// Configure the builder.
 				opts := []release.Option{
 					release.WithRepoRoot(cfg.RepoRootDir),
-					release.WithVersions(ver.FormattedString(), operatorVer.FormattedString()),
+					release.WithVersions(&version.Data{
+						ProductVersion:  ver,
+						OperatorVersion: operatorVer,
+					}),
 					release.WithOutputDir(filepath.Join(baseUploadDir, ver.FormattedString())),
 					release.WithArchitectures(cfg.Arches),
 					release.WithGithubOrg(cfg.Organization),
 				}
 				if c.Bool(skipValidationFlag) {
-					opts = append(opts, release.WithPreReleaseValidation(false))
+					opts = append(opts, release.WithValidate(false))
 				}
 				if reg := c.String(imageRegistryFlag); reg != "" {
 					opts = append(opts, release.WithImageRegistries([]string{reg}))
@@ -331,7 +357,10 @@ func releaseSubCommands(cfg *config.Config) []*cli.Command {
 				}
 				opts := []release.Option{
 					release.WithRepoRoot(cfg.RepoRootDir),
-					release.WithVersions(ver.FormattedString(), operatorVer.FormattedString()),
+					release.WithVersions(&version.Data{
+						ProductVersion:  ver,
+						OperatorVersion: operatorVer,
+					}),
 					release.WithOutputDir(filepath.Join(baseUploadDir, ver.FormattedString())),
 					release.WithPublishOptions(!c.Bool(skipPublishImagesFlag), !c.Bool(skipPublishGitTag), !c.Bool(skipPublishGithubRelease)),
 					release.WithGithubOrg(cfg.Organization),
@@ -381,7 +410,7 @@ func branchSubCommands(cfg *config.Config) []*cli.Command {
 				controller := operator.NewController(
 					operator.WithRepoRoot(cfg.OperatorConfig.Dir),
 					operator.WithRepoRemote(cfg.OperatorConfig.GitRemote),
-					operator.WithRepoOrganization(cfg.OperatorConfig.GitOrganization),
+					operator.WithGithubOrg(cfg.OperatorConfig.Organization),
 					operator.WithRepoName(cfg.OperatorConfig.GitRepository),
 					operator.WithBranch(utils.DefaultBranch),
 					operator.WithDevTagIdentifier(cfg.OperatorConfig.DevTagSuffix),

diff --git a/release/internal/command/git.go b/release/internal/command/git.go
@@ -1,22 +1,10 @@
 package command
 
-import (
-	"github.com/sirupsen/logrus"
-)
-
 // GitInDir runs a git command in a specific directory.
 func GitInDir(dir string, args ...string) (string, error) {
 	return runner().RunInDir(dir, "git", args, nil)
 }
 
-// GitInDir runs a git command in a specific directory
-// and fails if the command fails.
-func GitInDirOrFail(dir string, args ...string) {
-	if _, err := GitInDir(dir, args...); err != nil {
-		logrus.WithError(err).Fatal("Failed to run git command")
-	}
-}
-
 // Git runs a git command.
 func Git(args ...string) (string, error) {
 	return runner().Run("git", args, nil)

diff --git a/release/internal/hashrelease/pinnedversion.go b/release/internal/hashrelease/pinnedversion.go
@@ -59,6 +59,13 @@ func (c OperatorComponent) InitImage() Component {
 	}
 }
 
+type PinnedVersionConfig struct {
+	RootDir             string
+	ReleaseBranchPrefix string
+	DevTagSuffix        string
+	Operator            operator.Config
+}
+
 // PinnedVersionData represents the data needed to generate the pinned version file.
 type PinnedVersionData struct {
 	ReleaseName    string
@@ -93,22 +100,22 @@ func operatorComponentsFilePath(outputDir string) string {
 }
 
 // GeneratePinnedVersionFile generates the pinned version file.
-func GeneratePinnedVersionFile(rootDir, releaseBranchPrefix, devTagSuffix string, operatorConfig operator.Config, outputDir string) (string, *PinnedVersionData, error) {
+func GeneratePinnedVersionFile(cfg PinnedVersionConfig, outputDir string) (string, *PinnedVersionData, error) {
 	pinnedVersionPath := pinnedVersionFilePath(outputDir)
 
-	productBranch, err := utils.GitBranch(rootDir)
+	productBranch, err := utils.GitBranch(cfg.RootDir)
 	if err != nil {
 		return "", nil, err
 	}
 
-	productVersion := version.GitVersion(rootDir)
+	productVersion := version.GitVersion()
 	releaseName := fmt.Sprintf("%s-%s-%s", time.Now().Format("2006-01-02"), version.DeterminePublishStream(productBranch, string(productVersion)), RandomWord())
 	releaseName = strings.ReplaceAll(releaseName, ".", "-")
-	operatorBranch, err := operator.GitBranch(operatorConfig.Dir)
+	operatorBranch, err := operator.GitBranch(cfg.Operator.Dir)
 	if err != nil {
 		return "", nil, err
 	}
-	operatorVersion := version.GitVersion(operatorConfig.Dir)
+	operatorVersion := cfg.Operator.GitVersion()
 	tmpl, err := template.New("pinnedversion").Parse(pinnedVersionTemplateData)
 	if err != nil {
 		return "", nil, err
@@ -119,13 +126,13 @@ func GeneratePinnedVersionFile(rootDir, releaseBranchPrefix, devTagSuffix string
 		ProductVersion: productVersion.FormattedString(),
 		Operator: Component{
 			Version:  operatorVersion.FormattedString() + "-" + releaseName,
-			Image:    operatorConfig.Image,
-			Registry: operatorConfig.Registry,
+			Image:    cfg.Operator.Image,
+			Registry: cfg.Operator.Registry,
 		},
 		Hash: productVersion.FormattedString() + "-" + operatorVersion.FormattedString(),
 		Note: fmt.Sprintf("%s - generated at %s using %s release branch with %s operator branch",
 			releaseName, time.Now().Format(time.RFC1123), productBranch, operatorBranch),
-		ReleaseBranch: productVersion.ReleaseBranch(releaseBranchPrefix),
+		ReleaseBranch: productVersion.ReleaseBranch(cfg.ReleaseBranchPrefix),
 	}
 	logrus.WithField("file", pinnedVersionPath).Info("Generating pinned-version.yaml")
 	pinnedVersionFile, err := os.Create(pinnedVersionPath)

diff --git a/release/internal/operator/operator.go b/release/internal/operator/operator.go
@@ -2,14 +2,19 @@ package operator
 
 import (
 	"fmt"
+
+	"github.com/sirupsen/logrus"
+
+	"github.com/projectcalico/calico/release/internal/command"
+	"github.com/projectcalico/calico/release/internal/version"
 )
 
 type Config struct {
 	// GitRemote is the remote for the git repository
 	GitRemote string `envconfig:"OPERATOR_GIT_REMOTE" default:"origin"`
 
-	// GitOrganization is the organization for the operator
-	GitOrganization string `envconfig:"OPERATOR_GIT_ORGANIZATION" default:"tigera"`
+	// Organization is the GitHub organization for the operator
+	Organization string `envconfig:"OPERATOR_GIT_ORGANIZATION" default:"tigera"`
 
 	// GitRepository is the repository for the operator
 	GitRepository string `envconfig:"OPERATOR_GIT_REPOSITORY" default:"operator"`
@@ -34,7 +39,16 @@ type Config struct {
 }
 
 func (c Config) Repo() string {
-	return fmt.Sprintf("git@github.com:%s/%s.git", c.GitOrganization, c.GitRepository)
+	return fmt.Sprintf("git@github.com:%s/%s.git", c.Organization, c.GitRepository)
+}
+
+func (c Config) GitVersion() version.Version {
+	previousTag, err := command.GitVersion(c.Dir, true)
+	if err != nil {
+		logrus.WithError(err).Fatal("Failed to determine latest git version")
+	}
+	logrus.WithField("out", previousTag).Info("Current git describe")
+	return version.New(previousTag)
 }
 
 func (c Config) String() string {

diff --git a/release/internal/version/version.go b/release/internal/version/version.go
@@ -27,6 +27,14 @@ import (
 	"github.com/projectcalico/calico/release/internal/utils"
 )
 
+type Data struct {
+	// ProductVersion is the version of the product
+	ProductVersion Version
+
+	// OperatorVersion is the version of operator
+	OperatorVersion Version
+}
+
 // Version represents a version, and contains methods for working with versions.
 type Version string
 
@@ -74,10 +82,10 @@ func (v *Version) Semver() *semver.Version {
 	return ver
 }
 
-// GitVersion returns the current git version of the repository as a Version object.
-func GitVersion(dir string) Version {
+// GitVersion returns the current git version of the directory as a Version object.
+func GitVersion() Version {
 	// First, determine the git revision.
-	previousTag, err := command.GitVersion(dir, true)
+	previousTag, err := command.GitVersion(".", true)
 	if err != nil {
 		logrus.WithError(err).Fatal("Failed to determine latest git version")
 	}