Skip to content

Commit 9f0be4c

Browse files
committed
Code review 1 changes
Signed-off-by: Xavi Garcia <xavi.garcia@suse.com>
1 parent 7a0bd85 commit 9f0be4c

18 files changed

Lines changed: 251 additions & 101 deletions

File tree

charts/fleet-crd/templates/crds.yaml

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,12 @@ spec:
179179
a remote helm repository defined in a HelmOp resource'
180180
type: string
181181
type: object
182-
notInSchedule:
183-
description: 'NotInSchedule specifies if the BundleDeployment can
182+
ociContents:
183+
description: OCIContents is true when this deployment's contents
184+
is stored in an oci registry
185+
type: boolean
186+
offSchedule:
187+
description: 'OffSchedule specifies if the BundleDeployment can
184188
be updated.
185189
186190
If set to true, will stop any BundleDeployments from being
@@ -191,10 +195,6 @@ spec:
191195
192196
when changes are detected.'
193197
type: boolean
194-
ociContents:
195-
description: OCIContents is true when this deployment's contents
196-
is stored in an oci registry
197-
type: boolean
198198
options:
199199
description: Options are the deployment options, that are currently
200200
applied.
@@ -9326,7 +9326,7 @@ spec:
93269326
name: v1alpha1
93279327
schema:
93289328
openAPIV3Schema:
9329-
description: Schedule represents a schedule in which deployments are allower
9329+
description: Schedule represents a schedule in which deployments are allowed
93309330
or not, depending on its definition.
93319331
properties:
93329332
apiVersion:
@@ -9368,8 +9368,8 @@ spec:
93689368
properties:
93699369
clusters:
93709370
items:
9371-
description: ScheduleTarget represent a resource (or group
9372-
of resources affected by an Schedule)
9371+
description: ScheduleTarget represents a resource (or group
9372+
of resources) affected by a Schedule
93739373
properties:
93749374
clusterGroup:
93759375
description: ClusterGroup is the name of a cluster group

e2e/single-cluster/schedule_test.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"os"
99
"path"
1010
"strings"
11+
"time"
1112

1213
"github.com/go-git/go-git/v5"
1314
. "github.com/onsi/ginkgo/v2"
@@ -69,6 +70,11 @@ var _ = Describe("Schedules", Label("infra-setup"), func() {
6970
clonedir = path.Join(tmpDir, "test-repo")
7071

7172
gitrepoName = testenv.RandomFilename("gitjob-test", r)
73+
74+
// wait for the beginning of the next minute so we are sure
75+
// that the cycle checked in the tests is correct.
76+
// Note that the schedule starts at 0 seconds.
77+
waitForBeginningOfNextMinute()
7278
})
7379

7480
It("deploys and pauses based on the schedule", func() {
@@ -144,7 +150,7 @@ var _ = Describe("Schedules", Label("infra-setup"), func() {
144150
g.Expect(bdList.Items).To(HaveLen(1))
145151

146152
bd = bdList.Items[0]
147-
g.Expect(bd.Spec.NotInSchedule).To(BeFalse())
153+
g.Expect(bd.Spec.OffSchedule).To(BeFalse())
148154
}).Should(Succeed())
149155

150156
By("checking the pod exists")
@@ -164,7 +170,7 @@ var _ = Describe("Schedules", Label("infra-setup"), func() {
164170
return !cluster.Status.ActiveSchedule
165171
}).Should(BeTrue())
166172

167-
// When the schedule is inactive, the bundledeployment should be marked as NotInSchedule.
173+
// When the schedule is inactive, the bundledeployment should be marked as OffSchedule.
168174
By("checking the bundle deployment is paused")
169175
Eventually(func(g Gomega) {
170176
label := fmt.Sprintf("fleet.cattle.io/bundle-name=%s-examples", gitrepoName)
@@ -178,7 +184,7 @@ var _ = Describe("Schedules", Label("infra-setup"), func() {
178184
g.Expect(bdList.Items).To(HaveLen(1))
179185

180186
bd = bdList.Items[0]
181-
g.Expect(bd.Spec.NotInSchedule).To(BeTrue())
187+
g.Expect(bd.Spec.OffSchedule).To(BeTrue())
182188
}).Should(Succeed())
183189

184190
By("updating the git repository")
@@ -207,7 +213,7 @@ var _ = Describe("Schedules", Label("infra-setup"), func() {
207213
g.Expect(bdList.Items).To(HaveLen(1))
208214

209215
bd = bdList.Items[0]
210-
g.Expect(bd.Spec.NotInSchedule).To(BeTrue())
216+
g.Expect(bd.Spec.OffSchedule).To(BeTrue())
211217

212218
cluster, err = env.GetCluster(cluster.Name, cluster.Namespace)
213219
g.Expect(err).ToNot(HaveOccurred())
@@ -231,7 +237,7 @@ var _ = Describe("Schedules", Label("infra-setup"), func() {
231237
g.Expect(bdList.Items).To(HaveLen(1))
232238

233239
bd = bdList.Items[0]
234-
g.Expect(bd.Spec.NotInSchedule).To(BeFalse())
240+
g.Expect(bd.Spec.OffSchedule).To(BeFalse())
235241

236242
cluster, err = env.GetCluster(cluster.Name, cluster.Namespace)
237243
g.Expect(err).ToNot(HaveOccurred())
@@ -265,3 +271,13 @@ func getGitRepoStatus(g Gomega, k kubectl.Command, name string) fleet.GitRepoSta
265271

266272
return gitrepo.Status
267273
}
274+
275+
func waitForBeginningOfNextMinute() {
276+
for {
277+
now := time.Now()
278+
if now.Second() == 0 {
279+
break
280+
}
281+
time.Sleep(200 * time.Millisecond)
282+
}
283+
}

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ require (
4040
github.com/prometheus/client_model v0.6.2
4141
github.com/prometheus/common v0.65.0
4242
github.com/rancher/fleet/pkg/apis v0.13.0
43-
github.com/rancher/lasso v0.2.3
43+
github.com/rancher/lasso v0.2.4
4444
github.com/rancher/wrangler/v3 v3.2.4
45-
github.com/reugn/go-quartz v0.15.1
45+
github.com/reugn/go-quartz v0.15.2
4646
github.com/sirupsen/logrus v1.9.3
4747
github.com/spf13/cobra v1.10.1
4848
github.com/stretchr/testify v1.11.1

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,8 @@ github.com/prometheus/common v0.65.0 h1:QDwzd+G1twt//Kwj/Ww6E9FQq1iVMmODnILtW1t2
376376
github.com/prometheus/common v0.65.0/go.mod h1:0gZns+BLRQ3V6NdaerOhMbwwRbNh9hkGINtQAsP5GS8=
377377
github.com/prometheus/procfs v0.16.1 h1:hZ15bTNuirocR6u0JZ6BAHHmwS1p8B4P6MRqxtzMyRg=
378378
github.com/prometheus/procfs v0.16.1/go.mod h1:teAbpZRB1iIAJYREa1LsoWUXykVXA1KlTmWl8x/U+Is=
379-
github.com/rancher/lasso v0.2.3 h1:74/z/C/O3ykhyMrRuEgc9kVyYiSoS7kp5BAijlcyXDg=
380-
github.com/rancher/lasso v0.2.3/go.mod h1:G+KeeOaKRjp+qGp0bV6VbLhYrq1vHbJPbDh40ejg5yE=
379+
github.com/rancher/lasso v0.2.4 h1:rCoVo32q4cgUap2uyyzSLVdMeRJHBF/QOQwwSX/3oWI=
380+
github.com/rancher/lasso v0.2.4/go.mod h1:G+KeeOaKRjp+qGp0bV6VbLhYrq1vHbJPbDh40ejg5yE=
381381
github.com/rancher/wrangler/v3 v3.2.4 h1:pgpLwsmgQvTSSknxddJDq+ObIiOXFggCWdDyB0z7YcA=
382382
github.com/rancher/wrangler/v3 v3.2.4/go.mod h1:TA1QuuQxrtn/kmJbBLW/l24IcfHBmSXBa9an3IRlqQQ=
383383
github.com/redis/go-redis/extra/rediscmd/v9 v9.0.5 h1:EaDatTxkdHG+U3Bk4EUr+DZ7fOGwTfezUiUJMaIcaho=
@@ -386,8 +386,8 @@ github.com/redis/go-redis/extra/redisotel/v9 v9.0.5 h1:EfpWLLCyXw8PSM2/XNJLjI3Pb
386386
github.com/redis/go-redis/extra/redisotel/v9 v9.0.5/go.mod h1:WZjPDy7VNzn77AAfnAfVjZNvfJTYfPetfZk5yoSTLaQ=
387387
github.com/redis/go-redis/v9 v9.7.3 h1:YpPyAayJV+XErNsatSElgRZZVCwXX9QzkKYNvO7x0wM=
388388
github.com/redis/go-redis/v9 v9.7.3/go.mod h1:bGUrSggJ9X9GUmZpZNEOQKaANxSGgOEBRltRTZHSvrA=
389-
github.com/reugn/go-quartz v0.15.1 h1:8hMtC+ERa9G5tiL9fiuBpPreId+Tu1Xt2pkXVeTZAy0=
390-
github.com/reugn/go-quartz v0.15.1/go.mod h1:00DVnBKq2Fxag/HlR9mGXjmHNlMFQ1n/LNM+Fn0jUaE=
389+
github.com/reugn/go-quartz v0.15.2 h1:IQUnwTtNURVtdcwH4CJhFH3dXAUwP2fXZaNjPp+sJAY=
390+
github.com/reugn/go-quartz v0.15.2/go.mod h1:00DVnBKq2Fxag/HlR9mGXjmHNlMFQ1n/LNM+Fn0jUaE=
391391
github.com/rivo/uniseg v0.2.0 h1:S1pD9weZBuJdFmowNwbpi7BJ8TNftyUImj/0WQi72jY=
392392
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
393393
github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ=

internal/cmd/agent/controller/bundledeployment_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func (r *BundleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
126126

127127
return ctrl.Result{}, err
128128
}
129-
if bd.Spec.NotInSchedule {
129+
if bd.Spec.OffSchedule {
130130
logger.V(1).Info("Bundle not in schedule, clearing drift detection")
131131
err := r.DriftDetect.Clear(req.String())
132132

internal/cmd/agent/controller/drift_controller.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ func (r *DriftReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
8686

8787
return ctrl.Result{}, err
8888
}
89-
if bd.Spec.NotInSchedule {
89+
if bd.Spec.OffSchedule {
9090
logger.V(1).Info("Bundle not in schedule, clearing drift detection")
9191
err := r.DriftDetect.Clear(req.String())
9292

internal/cmd/controller/operator.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@ import (
99
"github.com/rancher/fleet/internal/cmd"
1010
"github.com/rancher/fleet/internal/cmd/controller/reconciler"
1111
"github.com/rancher/fleet/internal/cmd/controller/target"
12+
"github.com/rancher/fleet/internal/experimental"
1213
"github.com/rancher/fleet/internal/manifest"
1314
"github.com/rancher/fleet/internal/metrics"
1415
"github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
15-
"github.com/rancher/fleet/pkg/experimental"
1616

1717
"k8s.io/apimachinery/pkg/runtime"
1818
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -176,7 +176,7 @@ func start(
176176
Recorder: mgr.GetEventRecorderFor(fmt.Sprintf("fleet-schedule-ctrl%s", shardIDSuffix)),
177177
ShardID: shardID,
178178

179-
Workers: workersOpts.Bundle,
179+
Workers: workersOpts.Schedule,
180180
Scheduler: sched,
181181
}).SetupWithManager(mgr); err != nil {
182182
setupLog.Error(err, "unable to create controller", "controller", "Schedule")

internal/cmd/controller/reconciler/bundle_controller.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -754,8 +754,8 @@ func (r *BundleReconciler) cleanupOrphanedBundleDeployments(ctx context.Context,
754754
}
755755
toDelete := slices.DeleteFunc(list, func(bd fleet.BundleDeployment) bool {
756756
// don't delete BundleDeployments that are not in schedule as
757-
// that uninstall the deployment in the agent
758-
return uidsToKeep.Has(bd.UID) || bd.Spec.NotInSchedule
757+
// that would uninstall the deployment in the agent
758+
return uidsToKeep.Has(bd.UID) || bd.Spec.OffSchedule
759759
})
760760
return batchDeleteBundleDeployments(ctx, r.Client, toDelete)
761761
}

internal/cmd/controller/reconciler/cronduration_job.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ type CronDurationJob struct {
3030
key *quartz.JobKey
3131
}
3232

33-
// NewCronDurationJob constructs a new CronDurationJob.
33+
// newCronDurationJob constructs a new CronDurationJob.
3434
// It also verifies the validity and correctness of the schedule and duration data.
3535
// Internally, it assigns Location = Local if no location was specified in the schedule,
3636
// and from that point onward, any time-related calculations are performed using this location.
37-
func NewCronDurationJob(ctx context.Context, schedule *fleet.Schedule, scheduler quartz.Scheduler, c client.Client) (*CronDurationJob, error) {
37+
func newCronDurationJob(ctx context.Context, schedule *fleet.Schedule, scheduler quartz.Scheduler, c client.Client) (*CronDurationJob, error) {
3838
locationStr := schedule.Spec.Location
3939
if locationStr == "" {
4040
locationStr = "Local"
@@ -107,7 +107,7 @@ func getScheduleJobHash(sched *fleet.Schedule) (string, error) {
107107
return hex.EncodeToString(hash[:]), nil
108108
}
109109

110-
func (c *CronDurationJob) calculateNextTimeBasedOnCron() (time.Duration, error) {
110+
func (c *CronDurationJob) durationToNextStart() (time.Duration, error) {
111111
cronTrigger, err := quartz.NewCronTriggerWithLoc(c.Schedule.Spec.Schedule, c.Location)
112112
if err != nil {
113113
return 0, err
@@ -172,7 +172,7 @@ func (c *CronDurationJob) updateJob(ctx context.Context) error {
172172
}
173173

174174
func (c *CronDurationJob) rescheduleJob(ctx context.Context) error {
175-
next, err := c.calculateNextTimeBasedOnCron()
175+
next, err := c.durationToNextStart()
176176
if err != nil {
177177
return err
178178
}
@@ -231,6 +231,12 @@ func (c *CronDurationJob) executeStart(ctx context.Context) error {
231231
}
232232
}
233233

234+
// Sets Scheduled to true to all the matching clusters
235+
if err := setClustersScheduled(ctx, c.client, clusters, c.Schedule.Namespace, true); err != nil {
236+
return err
237+
}
238+
c.MatchingClusters = clusters
239+
234240
// Update the status of the Schedule resource
235241
if err := setScheduleActive(ctx, c.client, c.Schedule, true); err != nil {
236242
return err
@@ -243,7 +249,7 @@ func (c *CronDurationJob) executeStart(ctx context.Context) error {
243249
func (c *CronDurationJob) executeStop(ctx context.Context) error {
244250
c.Started = false
245251

246-
// Sets ActiveSchedule to true for all matching clusters.
252+
// Sets ActiveSchedule to false for all matching clusters.
247253
// This action disables the creation of BundleDeployments on the clusters.
248254
for _, cluster := range c.MatchingClusters {
249255
if err := setClusterActiveSchedule(context.Background(), c.client, cluster, c.Schedule.Namespace, false); err != nil {
@@ -271,7 +277,7 @@ func matchingClusters(ctx context.Context, matcher *matcher.ScheduleMatch, c cli
271277
cluster := cluster
272278
cgs, err := target.ClusterGroupsForCluster(ctx, c, &cluster)
273279
if err != nil {
274-
return nil, fmt.Errorf("%w, getting clusters from cluster groups: %w", fleetutil.ErrRetryable, err)
280+
return nil, fmt.Errorf("%w, getting cluster groups from clusters: %w", fleetutil.ErrRetryable, err)
275281
}
276282

277283
if matcher.MatchCluster(cluster.Name, target.ClusterGroupsToLabelMap(cgs), cluster.Labels) {

0 commit comments

Comments
 (0)