From 6eed76aa8e45cdb87a3ecd1248fd6b2c1d76b96b Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Mon, 5 Aug 2024 10:05:56 +0200 Subject: [PATCH] refactor: move package generation to a local variable Signed-off-by: Philip Laine --- src/pkg/packager/common.go | 7 ------- src/pkg/packager/deploy.go | 38 +++++++++++++++++++++----------------- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go index dd4588a193..6a3ec004e7 100644 --- a/src/pkg/packager/common.go +++ b/src/pkg/packager/common.go @@ -38,7 +38,6 @@ type Packager struct { hpaModified bool connectStrings types.ConnectStrings source sources.PackageSource - generation int } // Modifier is a function that modifies the packager. @@ -155,12 +154,6 @@ func (p *Packager) attemptClusterChecks(ctx context.Context) (err error) { spinner := message.NewProgressSpinner("Gathering additional cluster information (if available)") defer spinner.Stop() - // Check if the package has already been deployed and get its generation - if existingDeployedPackage, _ := p.cluster.GetDeployedPackage(ctx, p.cfg.Pkg.Metadata.Name); existingDeployedPackage != nil { - // If this package has been deployed before, increment the package generation within the secret - p.generation = existingDeployedPackage.Generation + 1 - } - // Check the clusters architecture matches the package spec if err := p.validatePackageArchitecture(ctx); err != nil { if errors.Is(err, lang.ErrUnableToCheckArch) { diff --git a/src/pkg/packager/deploy.go b/src/pkg/packager/deploy.go index 612fa5eb10..4f78959802 100644 --- a/src/pkg/packager/deploy.go +++ b/src/pkg/packager/deploy.go @@ -138,20 +138,8 @@ func (p *Packager) Deploy(ctx context.Context) error { // deployComponents loops through a list of ZarfComponents and deploys them. func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []types.DeployedComponent, err error) { - // Check if this package has been deployed before and grab relevant information about already deployed components - if p.generation == 0 { - p.generation = 1 // If this is the first deployment, set the generation to 1 - } - - // Process all the components we are deploying + // Connect to cluster if a component requires it. for _, component := range p.cfg.Pkg.Components { - deployedComponent := types.DeployedComponent{ - Name: component.Name, - Status: types.ComponentStatusDeploying, - ObservedGeneration: p.generation, - } - - // If this component requires a cluster, connect to one if component.RequiresCluster() { timeout := cluster.DefaultTimeout if p.cfg.Pkg.IsInitConfig() { @@ -160,8 +148,24 @@ func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []t connectCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() if err := p.connectToCluster(connectCtx); err != nil { - return deployedComponents, fmt.Errorf("unable to connect to the Kubernetes cluster: %w", err) + return nil, fmt.Errorf("unable to connect to the Kubernetes cluster: %w", err) } + break + } + } + + // If this package has been deployed before, increment the package generation within the secret + packageGeneration := 1 + if existingDeployedPackage, _ := p.cluster.GetDeployedPackage(ctx, p.cfg.Pkg.Metadata.Name); existingDeployedPackage != nil { + packageGeneration = existingDeployedPackage.Generation + 1 + } + + // Process all the components we are deploying + for _, component := range p.cfg.Pkg.Components { + deployedComponent := types.DeployedComponent{ + Name: component.Name, + Status: types.ComponentStatusDeploying, + ObservedGeneration: packageGeneration, } // Ensure we don't overwrite any installedCharts data when updating the package secret @@ -177,7 +181,7 @@ func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []t // Update the package secret to indicate that we are attempting to deploy this component if p.isConnectedToCluster() { - if _, err := p.cluster.RecordPackageDeploymentAndWait(ctx, p.cfg.Pkg, deployedComponents, p.connectStrings, p.generation, component, p.cfg.DeployOpts.SkipWebhooks); err != nil { + if _, err := p.cluster.RecordPackageDeploymentAndWait(ctx, p.cfg.Pkg, deployedComponents, p.connectStrings, packageGeneration, component, p.cfg.DeployOpts.SkipWebhooks); err != nil { message.Debugf("Unable to record package deployment for component %s: this will affect features like `zarf package remove`: %s", component.Name, err.Error()) } } @@ -205,7 +209,7 @@ func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []t // Update the package secret to indicate that we failed to deploy this component deployedComponents[idx].Status = types.ComponentStatusFailed if p.isConnectedToCluster() { - if _, err := p.cluster.RecordPackageDeploymentAndWait(ctx, p.cfg.Pkg, deployedComponents, p.connectStrings, p.generation, component, p.cfg.DeployOpts.SkipWebhooks); err != nil { + if _, err := p.cluster.RecordPackageDeploymentAndWait(ctx, p.cfg.Pkg, deployedComponents, p.connectStrings, packageGeneration, component, p.cfg.DeployOpts.SkipWebhooks); err != nil { message.Debugf("Unable to record package deployment for component %q: this will affect features like `zarf package remove`: %s", component.Name, err.Error()) } } @@ -217,7 +221,7 @@ func (p *Packager) deployComponents(ctx context.Context) (deployedComponents []t deployedComponents[idx].InstalledCharts = charts deployedComponents[idx].Status = types.ComponentStatusSucceeded if p.isConnectedToCluster() { - if _, err := p.cluster.RecordPackageDeploymentAndWait(ctx, p.cfg.Pkg, deployedComponents, p.connectStrings, p.generation, component, p.cfg.DeployOpts.SkipWebhooks); err != nil { + if _, err := p.cluster.RecordPackageDeploymentAndWait(ctx, p.cfg.Pkg, deployedComponents, p.connectStrings, packageGeneration, component, p.cfg.DeployOpts.SkipWebhooks); err != nil { message.Debugf("Unable to record package deployment for component %q: this will affect features like `zarf package remove`: %s", component.Name, err.Error()) } }