fix: improve behavior around cluster connection during deploy (#2088)

## Description Fixes #2085 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Other (security config, docs update, etc) ## Checklist before merging - [ ] Test, docs, adr added or updated as needed - [x] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed --------- Signed-off-by: razzle <[email protected]> Co-authored-by: Wayne Starr <[email protected]> Co-authored-by: Wayne Starr <[email protected]>
zarf-dev · Oct 26, 2023 · 1926a21 · 1926a21
1 parent 17a2cf9
commit 1926a21
Show file tree

Hide file tree

Showing 16 changed files with 291 additions and 173 deletions.
diff --git a/src/config/lang/english.go b/src/config/lang/english.go
@@ -259,8 +259,8 @@ const (
 	CmdPackageDeployFlagShasum                         = "Shasum of the package to deploy. Required if deploying a remote package and \"--insecure\" is not provided"
 	CmdPackageDeployFlagSget                           = "[Deprecated] Path to public sget key file for remote packages signed via cosign. This flag will be removed in v1.0.0 please use the --key flag instead."
 	CmdPackageDeployFlagSkipWebhooks                   = "[alpha] Skip waiting for external webhooks to execute as each package component is deployed"
-	CmdPackageDeployValidateArchitectureErr            = "this package architecture is %s, but the target cluster has the %s architecture. These architectures must be the same"
-	CmdPackageDeployValidateLastNonBreakingVersionWarn = "the version of this Zarf binary '%s' is less than the LastNonBreakingVersion of '%s'. You may need to upgrade your Zarf version to at least '%s' to deploy this package"
+	CmdPackageDeployValidateArchitectureErr            = "this package architecture is %s, but the target cluster only has the %s architecture(s). These architectures must be compatible when \"images\" are present"
+	CmdPackageDeployValidateLastNonBreakingVersionWarn = "The version of this Zarf binary '%s' is less than the LastNonBreakingVersion of '%s'. You may need to upgrade your Zarf version to at least '%s' to deploy this package"
 	CmdPackageDeployInvalidCLIVersionWarn              = "CLIVersion is set to '%s' which can cause issues with package creation and deployment. To avoid such issues, please set the value to the valid semantic version for this version of Zarf."
 	CmdPackageDeployErr                                = "Failed to deploy package: %s"
 

diff --git a/src/internal/cluster/state.go b/src/internal/cluster/state.go
@@ -35,19 +35,13 @@ const (
 // InitZarfState initializes the Zarf state with the given temporary directory and init configs.
 func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error {
 	var (
-		clusterArch string
-		distro      string
-		err         error
+		distro string
+		err    error
 	)
 
-	spinner := message.NewProgressSpinner("Gathering cluster information")
+	spinner := message.NewProgressSpinner("Gathering cluster state information")
 	defer spinner.Stop()
 
-	spinner.Updatef("Getting cluster architecture")
-	if clusterArch, err = c.GetArchitecture(); err != nil {
-		spinner.Errorf(err, "Unable to validate the cluster system architecture")
-	}
-
 	// Attempt to load an existing state prior to init.
 	// NOTE: We are ignoring the error here because we don't really expect a state to exist yet.
 	spinner.Updatef("Checking cluster for existing Zarf deployment")
@@ -77,7 +71,6 @@ func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error {
 
 		// Defaults
 		state.Distro = distro
-		state.Architecture = clusterArch
 		state.LoggingSecret = utils.RandomString(config.ZarfGeneratedPasswordLen)
 
 		// Setup zarf agent PKI
@@ -134,10 +127,6 @@ func (c *Cluster) InitZarfState(initOptions types.ZarfInitOptions) error {
 		}
 	}
 
-	if clusterArch != state.Architecture {
-		return fmt.Errorf("cluster architecture %s does not match the Zarf state architecture %s", clusterArch, state.Architecture)
-	}
-
 	switch state.Distro {
 	case k8s.DistroIsK3s, k8s.DistroIsK3d:
 		state.StorageClass = "local-path"

diff --git a/src/pkg/k8s/info.go b/src/pkg/k8s/info.go
@@ -114,18 +114,30 @@ func (k *K8s) DetectDistro() (string, error) {
 	return DistroIsUnknown, nil
 }
 
-// GetArchitecture returns the cluster system architecture if found or an error if not.
-func (k *K8s) GetArchitecture() (string, error) {
+// GetArchitectures returns the cluster system architectures if found.
+func (k *K8s) GetArchitectures() ([]string, error) {
 	nodes, err := k.GetNodes()
 	if err != nil {
-		return "", err
+		return nil, err
 	}
 
+	if len(nodes.Items) == 0 {
+		return nil, errors.New("could not identify node architecture")
+	}
+
+	archMap := map[string]bool{}
+
 	for _, node := range nodes.Items {
-		return node.Status.NodeInfo.Architecture, nil
+		archMap[node.Status.NodeInfo.Architecture] = true
+	}
+
+	architectures := []string{}
+
+	for arch := range archMap {
+		architectures = append(architectures, arch)
 	}
 
-	return "", errors.New("could not identify node architecture")
+	return architectures, nil
 }
 
 // GetServerVersion retrieves and returns the k8s revision.

diff --git a/src/pkg/packager/common.go b/src/pkg/packager/common.go
@@ -6,11 +6,13 @@ package packager
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
+	"time"
 
 	"github.com/Masterminds/semver/v3"
 	"github.com/defenseunicorns/zarf/src/config/lang"
@@ -19,12 +21,14 @@ import (
 	"github.com/defenseunicorns/zarf/src/internal/packager/template"
 	"github.com/defenseunicorns/zarf/src/types"
 	"github.com/mholt/archiver/v3"
+	"k8s.io/utils/strings/slices"
 
 	"github.com/defenseunicorns/zarf/src/config"
 	"github.com/defenseunicorns/zarf/src/pkg/interactive"
 	"github.com/defenseunicorns/zarf/src/pkg/layout"
 	"github.com/defenseunicorns/zarf/src/pkg/message"
 	"github.com/defenseunicorns/zarf/src/pkg/oci"
+	"github.com/defenseunicorns/zarf/src/pkg/packager/deprecated"
 	"github.com/defenseunicorns/zarf/src/pkg/packager/sources"
 	"github.com/defenseunicorns/zarf/src/pkg/utils"
 )
@@ -174,7 +178,7 @@ func GetInitPackageName(arch string) string {
 
 // GetPackageName returns the formatted name of the package.
 func (p *Packager) GetPackageName() string {
-	if p.cfg.Pkg.Kind == types.ZarfInitConfig {
+	if p.isInitConfig() {
 		return GetInitPackageName(p.arch)
 	}
 
@@ -201,21 +205,90 @@ func (p *Packager) ClearTempPaths() {
 	_ = os.RemoveAll(layout.SBOMDir)
 }
 
+// connectToCluster attempts to connect to a cluster if a connection is not already established
+func (p *Packager) connectToCluster(timeout time.Duration) (err error) {
+	if p.isConnectedToCluster() {
+		return nil
+	}
+
+	p.cluster, err = cluster.NewClusterWithWait(timeout)
+	if err != nil {
+		return err
+	}
+
+	return p.attemptClusterChecks()
+}
+
+// isConnectedToCluster returns whether the current packager instance is connected to a cluster
+func (p *Packager) isConnectedToCluster() bool {
+	return p.cluster != nil
+}
+
+// isInitConfig returns whether the current packager instance is deploying an init config
+func (p *Packager) isInitConfig() bool {
+	return p.cfg.Pkg.Kind == types.ZarfInitConfig
+}
+
+// hasImages returns whether the current package contains images
+func (p *Packager) hasImages() bool {
+	for _, component := range p.cfg.Pkg.Components {
+		if len(component.Images) > 0 {
+			return true
+		}
+	}
+	return false
+}
+
+// attemptClusterChecks attempts to connect to the cluster and check for useful metadata and config mismatches.
+// NOTE: attemptClusterChecks should only return an error if there is a problem significant enough to halt a deployment, otherwise it should return nil and print a warning message.
+func (p *Packager) attemptClusterChecks() (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(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(); err != nil {
+		if errors.Is(err, lang.ErrUnableToCheckArch) {
+			message.Warnf("Unable to validate package architecture: %s", err.Error())
+		} else {
+			return err
+		}
+	}
+
+	// Check for any breaking changes between the initialized Zarf version and this CLI
+	if existingInitPackage, _ := p.cluster.GetDeployedPackage("init"); existingInitPackage != nil {
+		// Use the build version instead of the metadata since this will support older Zarf versions
+		deprecated.PrintBreakingChanges(existingInitPackage.Data.Build.Version)
+	} else {
+		message.Warnf("Unable to retrieve the initialized Zarf version.  There is potential for breaking changes.")
+	}
+
+	spinner.Success()
+
+	return nil
+}
+
 // validatePackageArchitecture validates that the package architecture matches the target cluster architecture.
 func (p *Packager) validatePackageArchitecture() error {
-	// Ignore this check if the architecture is explicitly "multi" or we don't have a cluster connection
-	if p.arch == "multi" || p.cluster == nil {
+	// Ignore this check if the architecture is explicitly "multi", we don't have a cluster connection, or the package contains no images
+	if p.arch == "multi" || !p.isConnectedToCluster() || !p.hasImages() {
 		return nil
 	}
 
-	clusterArch, err := p.cluster.GetArchitecture()
+	clusterArchitectures, err := p.cluster.GetArchitectures()
 	if err != nil {
 		return lang.ErrUnableToCheckArch
 	}
 
 	// Check if the package architecture and the cluster architecture are the same.
-	if p.arch != clusterArch {
-		return fmt.Errorf(lang.CmdPackageDeployValidateArchitectureErr, p.arch, clusterArch)
+	if !slices.Contains(clusterArchitectures, p.arch) {
+		return fmt.Errorf(lang.CmdPackageDeployValidateArchitectureErr, p.arch, strings.Join(clusterArchitectures, ", "))
 	}
 
 	return nil

diff --git a/src/pkg/packager/common_test.go b/src/pkg/packager/common_test.go
@@ -24,7 +24,8 @@ func TestValidatePackageArchitecture(t *testing.T) {
 	type testCase struct {
 		name          string
 		pkgArch       string
-		clusterArch   string
+		clusterArchs  []string
+		images        []string
 		expectedError error
 		getArchError  error
 	}
@@ -33,24 +34,41 @@ func TestValidatePackageArchitecture(t *testing.T) {
 		{
 			name:          "architecture match",
 			pkgArch:       "amd64",
-			clusterArch:   "amd64",
+			clusterArchs:  []string{"amd64"},
+			images:        []string{"nginx"},
 			expectedError: nil,
 		},
 		{
 			name:          "architecture mismatch",
 			pkgArch:       "arm64",
-			clusterArch:   "amd64",
+			clusterArchs:  []string{"amd64"},
+			images:        []string{"nginx"},
 			expectedError: fmt.Errorf(lang.CmdPackageDeployValidateArchitectureErr, "arm64", "amd64"),
 		},
+		{
+			name:          "multiple cluster architectures",
+			pkgArch:       "arm64",
+			clusterArchs:  []string{"amd64", "arm64"},
+			images:        []string{"nginx"},
+			expectedError: nil,
+		},
 		{
 			name:          "ignore validation when package arch equals 'multi'",
 			pkgArch:       "multi",
-			clusterArch:   "not evaluated",
+			clusterArchs:  []string{"not evaluated"},
+			expectedError: nil,
+		},
+		{
+			name:          "ignore validation when a package doesn't contain images",
+			pkgArch:       "amd64",
+			images:        []string{},
+			clusterArchs:  []string{"not evaluated"},
 			expectedError: nil,
 		},
 		{
 			name:          "test the error path when fetching cluster architecture fails",
 			pkgArch:       "amd64",
+			images:        []string{"nginx"},
 			getArchError:  errors.New("error fetching cluster architecture"),
 			expectedError: lang.ErrUnableToCheckArch,
 		},
@@ -74,6 +92,15 @@ func TestValidatePackageArchitecture(t *testing.T) {
 						Log:       logger,
 					},
 				},
+				cfg: &types.PackagerConfig{
+					Pkg: types.ZarfPackage{
+						Components: []types.ZarfComponent{
+							{
+								Images: testCase.images,
+							},
+						},
+					},
+				},
 			}
 
 			// Set up test data for fetching cluster architecture.
@@ -83,17 +110,21 @@ func TestValidatePackageArchitecture(t *testing.T) {
 					return true, nil, testCase.getArchError
 				}
 
-				// Create a NodeList object to fetch cluster architecture with the mock client.
-				nodeList := &v1.NodeList{
-					Items: []v1.Node{
-						{
-							Status: v1.NodeStatus{
-								NodeInfo: v1.NodeSystemInfo{
-									Architecture: testCase.clusterArch,
-								},
+				nodeItems := []v1.Node{}
+
+				for _, arch := range testCase.clusterArchs {
+					nodeItems = append(nodeItems, v1.Node{
+						Status: v1.NodeStatus{
+							NodeInfo: v1.NodeSystemInfo{
+								Architecture: arch,
 							},
 						},
-					},
+					})
+				}
+
+				// Create a NodeList object to fetch cluster architecture with the mock client.
+				nodeList := &v1.NodeList{
+					Items: nodeItems,
 				}
 				return true, nodeList, nil
 			})

diff --git a/src/pkg/packager/components.go b/src/pkg/packager/components.go
@@ -35,11 +35,11 @@ func (p *Packager) getValidComponents() []types.ZarfComponent {
 			key = component.Name
 		} else {
 			// Otherwise, add the component name to the choice group list for later validation
-			choiceComponents = p.appendIfNotExists(choiceComponents, component.Name)
+			choiceComponents = helpers.AppendIfNotExists(choiceComponents, component.Name)
 		}
 
 		// Preserve component order
-		orderedKeys = p.appendIfNotExists(orderedKeys, key)
+		orderedKeys = helpers.AppendIfNotExists(orderedKeys, key)
 
 		// Append the component to the list of components in the group
 		componentGroups[key] = append(componentGroups[key], component)
@@ -65,7 +65,7 @@ func (p *Packager) getValidComponents() []types.ZarfComponent {
 
 			if requested {
 				// Mark deployment as appliance mode if this is an init config and the k3s component is enabled
-				if component.Name == k8s.DistroIsK3s && p.cfg.Pkg.Kind == types.ZarfInitConfig {
+				if component.Name == k8s.DistroIsK3s && p.isInitConfig() {
 					p.cfg.InitOpts.ApplianceMode = true
 				}
 				// Add the component to the list of valid components
@@ -208,15 +208,6 @@ func (p *Packager) confirmChoiceGroup(componentGroup []types.ZarfComponent) type
 	return componentGroup[chosen]
 }
 
-func (p *Packager) appendIfNotExists(slice []string, item string) []string {
-	for _, s := range slice {
-		if s == item {
-			return slice
-		}
-	}
-	return append(slice, item)
-}
-
 func (p *Packager) requiresCluster(component types.ZarfComponent) bool {
 	hasImages := len(component.Images) > 0
 	hasCharts := len(component.Charts) > 0

diff --git a/src/pkg/packager/create.go b/src/pkg/packager/create.go
@@ -54,7 +54,7 @@ func (p *Packager) Create() (err error) {
 	}
 	message.Note(fmt.Sprintf("Using build directory %s", p.cfg.CreateOpts.BaseDir))
 
-	if p.cfg.Pkg.Kind == types.ZarfInitConfig {
+	if p.isInitConfig() {
 		p.cfg.Pkg.Metadata.Version = config.CLIVersion
 	}