Skip to content

Commit aa139cc

Browse files
weyfonkweyfonk
committed
[WIP] Detect overlaps between bundles of a same GitRepo
This commit enables `fleet apply` to detect any possible resource overlaps, by kind and name, between: * obsolete bundles, which will be deleted because they no longer match any path in the GitRepo * new bundles coming from the GitRepo, to be created When such overlaps are detected, the bundle to be written has a newly populated `Overwrites` field, which is propagated to bundle deployments when targeting that bundle, specifying which resources (name, kind and namespace) this bundle deployment will overwrite from other bundles. This enables the agent to detect if a bundle deployment's `ModifiedStatus` features missing resources which may have been deleted when deleting a previous, obsolete bundle, for the bundle deployment to be redeployed. This last part does not quite work yet: * new deployments are triggered more than once * they do not result in updated, non-modified bundle deployment statuses. As a side note, it is unclear whether namespaces can even be reliably populated in bundle deployment overwrites, as a bundle resource's actual target namespace may depend on multiple factors.
1 parent 56baa24 commit aa139cc

8 files changed

Lines changed: 357 additions & 14 deletions

File tree

‎charts/fleet-crd/templates/crds.yaml‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,22 @@ spec:
518518
to the namespace created by Fleet.
519519
nullable: true
520520
type: object
521+
overwrites:
522+
description: 'Overwrites indicates which resources, if any,
523+
come from this bundle and overwrite another existing bundle.
524+
525+
This flag is set internally by Fleet, and should not be altered
526+
by users.'
527+
items:
528+
properties:
529+
kind:
530+
type: string
531+
name:
532+
type: string
533+
namespace:
534+
type: string
535+
type: object
536+
type: array
521537
serviceAccount:
522538
description: ServiceAccount which will be used to perform this
523539
deployment.
@@ -888,6 +904,22 @@ spec:
888904
to the namespace created by Fleet.
889905
nullable: true
890906
type: object
907+
overwrites:
908+
description: 'Overwrites indicates which resources, if any,
909+
come from this bundle and overwrite another existing bundle.
910+
911+
This flag is set internally by Fleet, and should not be altered
912+
by users.'
913+
items:
914+
properties:
915+
kind:
916+
type: string
917+
name:
918+
type: string
919+
namespace:
920+
type: string
921+
type: object
922+
type: array
891923
serviceAccount:
892924
description: ServiceAccount which will be used to perform this
893925
deployment.
@@ -1809,6 +1841,22 @@ spec:
18091841
the namespace created by Fleet.
18101842
nullable: true
18111843
type: object
1844+
overwrites:
1845+
description: 'Overwrites indicates which resources, if any, come
1846+
from this bundle and overwrite another existing bundle.
1847+
1848+
This flag is set internally by Fleet, and should not be altered
1849+
by users.'
1850+
items:
1851+
properties:
1852+
kind:
1853+
type: string
1854+
name:
1855+
type: string
1856+
namespace:
1857+
type: string
1858+
type: object
1859+
type: array
18121860
paused:
18131861
description: Paused if set to true, will stop any BundleDeployments
18141862
from being updated. It will be marked as out of sync.
@@ -2688,6 +2736,22 @@ spec:
26882736
to the namespace created by Fleet.
26892737
nullable: true
26902738
type: object
2739+
overwrites:
2740+
description: 'Overwrites indicates which resources, if any,
2741+
come from this bundle and overwrite another existing bundle.
2742+
2743+
This flag is set internally by Fleet, and should not be
2744+
altered by users.'
2745+
items:
2746+
properties:
2747+
kind:
2748+
type: string
2749+
name:
2750+
type: string
2751+
namespace:
2752+
type: string
2753+
type: object
2754+
type: array
26912755
serviceAccount:
26922756
description: ServiceAccount which will be used to perform
26932757
this deployment.
@@ -7719,6 +7783,22 @@ spec:
77197783
the namespace created by Fleet.
77207784
nullable: true
77217785
type: object
7786+
overwrites:
7787+
description: 'Overwrites indicates which resources, if any, come
7788+
from this bundle and overwrite another existing bundle.
7789+
7790+
This flag is set internally by Fleet, and should not be altered
7791+
by users.'
7792+
items:
7793+
properties:
7794+
kind:
7795+
type: string
7796+
name:
7797+
type: string
7798+
namespace:
7799+
type: string
7800+
type: object
7801+
type: array
77227802
paused:
77237803
description: Paused if set to true, will stop any BundleDeployments
77247804
from being updated. It will be marked as out of sync.
@@ -8603,6 +8683,22 @@ spec:
86038683
to the namespace created by Fleet.
86048684
nullable: true
86058685
type: object
8686+
overwrites:
8687+
description: 'Overwrites indicates which resources, if any,
8688+
come from this bundle and overwrite another existing bundle.
8689+
8690+
This flag is set internally by Fleet, and should not be
8691+
altered by users.'
8692+
items:
8693+
properties:
8694+
kind:
8695+
type: string
8696+
name:
8697+
type: string
8698+
namespace:
8699+
type: string
8700+
type: object
8701+
type: array
86068702
serviceAccount:
86078703
description: ServiceAccount which will be used to perform
86088704
this deployment.

‎integrationtests/cli/apply/apply_online_test.go‎

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,14 @@ import (
2323
var _ = Describe("Fleet apply online", Label("online"), func() {
2424

2525
var (
26-
ctrl *gomock.Controller
27-
clientMock *mocks.MockK8sClient
28-
name string
29-
dirs []string
30-
options apply.Options
31-
oldBundle *fleet.Bundle
32-
newBundle *fleet.Bundle
26+
ctrl *gomock.Controller
27+
clientMock *mocks.MockK8sClient
28+
name string
29+
dirs []string
30+
options apply.Options
31+
oldBundle *fleet.Bundle
32+
newBundle *fleet.Bundle
33+
listedBundles []fleet.Bundle
3334
)
3435

3536
JustBeforeEach(func() {
@@ -47,7 +48,14 @@ var _ = Describe("Fleet apply online", Label("online"), func() {
4748
return nil
4849
},
4950
).AnyTimes()
50-
clientMock.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
51+
// Set listedBundles to a non-empty list to test bundle pruning behaviour (pruneBundlesNotFoundInRepo)
52+
clientMock.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
53+
func(_ context.Context, l *fleet.BundleList, _ ...interface{}) error {
54+
l.Items = listedBundles
55+
56+
return nil
57+
},
58+
).AnyTimes()
5159
clientMock.EXPECT().Delete(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
5260
// so it does not try to use OCI storage
5361
clientMock.EXPECT().Get(
@@ -145,4 +153,57 @@ data:
145153
Expect(err.Error()).To(ContainSubstring("already exists"))
146154
})
147155
})
156+
157+
When("A bundle with a different name and overlapping resources already exists in the same namespace", func() {
158+
BeforeEach(func() {
159+
name = "labels_update"
160+
dirs = []string{cli.AssetsPath + "labels_update"}
161+
//bundle in the cluster
162+
oldBundle = &fleet.Bundle{
163+
ObjectMeta: metav1.ObjectMeta{
164+
Namespace: "foo",
165+
Name: "labels_update",
166+
},
167+
Spec: fleet.BundleSpec{
168+
Resources: []fleet.BundleResource{
169+
{
170+
Name: "cm.yaml",
171+
Content: `apiVersion: v1
172+
kind: ConfigMap
173+
metadata:
174+
name: cm3
175+
data:
176+
test: "value34"`, // same name, same kind, contents should not matter
177+
},
178+
{
179+
Name: "chart/values.yaml",
180+
Content: "name: ignore-missing-resource",
181+
},
182+
},
183+
},
184+
}
185+
186+
listedBundles = []fleet.Bundle{*oldBundle}
187+
})
188+
189+
It("detects the existing bundle and populates Overwrites on the new bundle", func() {
190+
expectedOverwrites := []fleet.OverwrittenResource{
191+
{
192+
Kind: "ConfigMap",
193+
Name: "cm3",
194+
},
195+
}
196+
197+
clientMock.EXPECT().Update(gomock.Any(), gomock.AssignableToTypeOf(&fleet.Bundle{}), gomock.Any()).DoAndReturn(
198+
func(_ context.Context, b *fleet.Bundle, _ ...interface{}) error {
199+
Expect(b).NotTo(BeNil())
200+
Expect(b.Spec.Overwrites).To(Equal(expectedOverwrites))
201+
return nil
202+
},
203+
)
204+
205+
err := fleetApplyOnline(clientMock, name, dirs, options)
206+
Expect(err).NotTo(HaveOccurred())
207+
})
208+
})
148209
})

‎internal/cmd/agent/controller/bundledeployment_controller.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/rancher/fleet/internal/cmd/agent/deployer/monitor"
1212
"github.com/rancher/fleet/internal/helmvalues"
1313
fleetv1 "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
14+
"github.com/rancher/fleet/pkg/durations"
1415

1516
corev1 "k8s.io/api/core/v1"
1617
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -205,6 +206,28 @@ func (r *BundleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
205206
merr = append(merr, fmt.Errorf("failed refreshing drift detection: %w", err))
206207
}
207208

209+
logger.V(1).Info("### ModifiedStatus", "orig", orig.Status.ModifiedStatus, "bd", bd.Status.ModifiedStatus)
210+
// FIXME why would this run multiple times, e.g. every 5s?
211+
if len(orig.Status.ModifiedStatus) > 0 && len(orig.Spec.Options.Overwrites) > 0 {
212+
for _, ms := range orig.Status.ModifiedStatus {
213+
if !ms.Create { // missing
214+
continue
215+
}
216+
for _, ow := range orig.Spec.Options.Overwrites {
217+
if ow.Kind == ms.Kind && ow.Name == ms.Name {
218+
logger.V(1).Info(
219+
"Triggering new deployment to overwrite missing resource",
220+
"kind", ow.Kind,
221+
"name", ow.Name,
222+
"namespace", ow.Namespace,
223+
)
224+
225+
return ctrl.Result{RequeueAfter: durations.DefaultRequeueAfter}, nil
226+
}
227+
}
228+
}
229+
}
230+
208231
if err := r.Cleanup.CleanupReleases(ctx, key, bd); err != nil {
209232
logger.V(1).Info("Failed to clean up bundledeployment releases", "error", err)
210233
}

0 commit comments

Comments
 (0)