Skip to content

Commit a8dd88f

Browse files
authored
Optimise target building and fix panic in integration tests (#4802)
* Compute cluster groups label map only once The same label map does not need to be computed three times for the same cluster groups. * Fix integration tests This fixes a panic in integration tests for `doNotDeploy`, and ensures that the `It` block title is formatted like a sentence, for better readability.
1 parent 847d54e commit a8dd88f

File tree

2 files changed

+9
-4
lines changed

2 files changed

+9
-4
lines changed

integrationtests/controller/bundle/bundle_targets_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ var _ = Describe("Bundle targets", Ordered, func() {
398398
targets = append(targets, targetsInGitRepo...)
399399
})
400400

401-
It("no BundleDeployments are created", func() {
401+
It("does not create any BundleDeployments", func() {
402402
waitForBundleToBeReady(bundleName)
403403
_ = verifyBundlesDeploymentsAreCreated(expectedNumberOfBundleDeployments, bdLabels, bundleName)
404404
})
@@ -592,6 +592,9 @@ var _ = Describe("Bundle targets", Ordered, func() {
592592

593593
func verifyBundlesDeploymentsAreCreated(numBundleDeployments int, bdLabels map[string]string, bundleName string) *v1alpha1.BundleDeploymentList {
594594
var bdList *v1alpha1.BundleDeploymentList
595+
if bdLabels == nil {
596+
bdLabels = map[string]string{}
597+
}
595598
bdLabels["fleet.cattle.io/bundle-name"] = bundleName
596599
Eventually(func() int {
597600
bdList = &v1alpha1.BundleDeploymentList{}

internal/cmd/controller/target/builder.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,16 @@ func (m *Manager) Targets(ctx context.Context, bundle *fleet.Bundle, manifestID
7979
return nil, false, err
8080
}
8181

82-
target := bm.Match(cluster.Name, ClusterGroupsToLabelMap(clusterGroups), cluster.Labels)
82+
clusterGroupsAsLabelMap := ClusterGroupsToLabelMap(clusterGroups)
83+
84+
target := bm.Match(cluster.Name, clusterGroupsAsLabelMap, cluster.Labels)
8385
if target == nil {
8486
continue
8587
}
8688
// Check all matching targetCustomizations for doNotDeploy, not just the first match.
8789
// This ensures that a doNotDeploy entry is honoured even when a broader-matching
8890
// target appears before it in the target list (fixes first-match bypass).
89-
doNotDeploy := target.DoNotDeploy || bm.HasDoNotDeployTarget(cluster.Name, ClusterGroupsToLabelMap(clusterGroups), cluster.Labels)
91+
doNotDeploy := target.DoNotDeploy || bm.HasDoNotDeployTarget(cluster.Name, clusterGroupsAsLabelMap, cluster.Labels)
9092
if doNotDeploy {
9193
logger.V(1).Info("Skipping BundleDeployment creation because doNotDeploy is set to true.",
9294
"bundle", bundle.Name,
@@ -99,7 +101,7 @@ func (m *Manager) Targets(ctx context.Context, bundle *fleet.Bundle, manifestID
99101
}
100102
// check if there is any matching targetCustomization that should be applied
101103
targetOpts := target.BundleDeploymentOptions
102-
targetCustomized := bm.MatchTargetCustomizations(cluster.Name, ClusterGroupsToLabelMap(clusterGroups), cluster.Labels)
104+
targetCustomized := bm.MatchTargetCustomizations(cluster.Name, clusterGroupsAsLabelMap, cluster.Labels)
103105
if targetCustomized != nil {
104106
targetOpts = targetCustomized.BundleDeploymentOptions
105107
}

0 commit comments

Comments
 (0)