Skip to content

Commit 32d912e

Browse files
committed
address comments
Signed-off-by: Zhiying Lin <[email protected]>
1 parent cfdab36 commit 32d912e

File tree

14 files changed

+93
-93
lines changed

14 files changed

+93
-93
lines changed

.github/workflows/ci.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,11 @@ jobs:
9393
include:
9494
- customized-settings: default
9595
# to shorten the test duration, set the resource snapshot creation interval to 0
96-
resource-snapshot-creation-interval: 0m
96+
resource-snapshot-creation-minimum-interval: 0m
9797
resource-changes-collection-duration: 0m
9898
- customized-settings: custom
99-
resource-snapshot-creation-interval: 30s
100-
resource-changes-collection-duration: 30s
99+
resource-snapshot-creation-minimum-interval: 30s
100+
resource-changes-collection-duration: 15s
101101
runs-on: ubuntu-latest
102102
needs: [
103103
detect-noop,
@@ -145,6 +145,6 @@ jobs:
145145
# TO-DO (chenyu1): to ensure a vendor-neutral experience, switch to a dummy
146146
# property provider once the AKS one is split out.
147147
PROPERTY_PROVIDER: 'azure'
148-
RESOURCE_SNAPSHOT_CREATION_INTERVAL: ${{ matrix.resource-snapshot-creation-interval }}
148+
RESOURCE_SNAPSHOT_CREATION_MINIMUM_INTERVAL: ${{ matrix.resource-snapshot-creation-minimum-interval }}
149149
RESOURCE_CHANGES_COLLECTION_DURATION: ${{ matrix.resource-changes-collection-duration }}
150150

charts/hub-agent/README.md

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,26 @@ _See [helm install](https://helm.sh/docs/helm/helm_install/) for command documen
1919

2020
## Parameters
2121

22-
| Parameter | Description | Default |
23-
|:----------------------------------|:-----------------------------------------------------------------------------------------------------------------------------------------------------------|:-------------------------------------------------|
24-
| `replicaCount` | Number of hub-agent replicas to deploy | `1` |
25-
| `image.repository` | Image repository | `ghcr.io/azure/azure/fleet/hub-agent` |
26-
| `image.pullPolicy` | Image pull policy | `Always` |
27-
| `image.tag` | Image release tag | `v0.1.0` |
28-
| `namespace` | Namespace where this chart is installed | `fleet-system` |
29-
| `serviceAccount.create` | Whether to create a service account | `true` |
30-
| `serviceAccount.name` | Service account name | `hub-agent-sa` |
31-
| `resources` | Resource requests/limits for the container | limits: 500m CPU, 1Gi; requests: 100m CPU, 128Mi |
32-
| `affinity` | Node affinity for hub-agent pods | `{}` |
33-
| `tolerations` | Tolerations for hub-agent pods | `[]` |
34-
| `logVerbosity` | Log level (klog V logs) | `5` |
35-
| `enableV1Alpha1APIs` | Watch for v1alpha1 APIs | `false` |
36-
| `enableV1Beta1APIs` | Watch for v1beta1 APIs | `true` |
37-
| `hubAPIQPS` | QPS for fleet-apiserver (not including events/node heartbeat) | `250` |
38-
| `hubAPIBurst` | Burst for fleet-apiserver (not including events/node heartbeat) | `1000` |
39-
| `MaxConcurrentClusterPlacement` | Max concurrent ClusterResourcePlacement operations | `100` |
40-
| `ConcurrentResourceChangeSyncs` | Max concurrent resourceChange reconcilers | `20` |
41-
| `logFileMaxSize` | Max log file size before rotation | `1000000` |
42-
| `MaxFleetSizeSupported` | Max number of member clusters supported | `100` |
43-
| `resourceSnapshotCreationInterval` | Interval for resource snapshot creation | `30s` |
44-
| `resourceChangesCollectionDuration` | Interval for resource snapshot creation | `30s`
22+
| Parameter | Description | Default |
23+
|:------------------------------------------|:------------------------------------------------------------------------------------------|:-------------------------------------------------|
24+
| `replicaCount` | Number of hub-agent replicas to deploy | `1` |
25+
| `image.repository` | Image repository | `ghcr.io/azure/azure/fleet/hub-agent` |
26+
| `image.pullPolicy` | Image pull policy | `Always` |
27+
| `image.tag` | Image release tag | `v0.1.0` |
28+
| `namespace` | Namespace where this chart is installed | `fleet-system` |
29+
| `serviceAccount.create` | Whether to create a service account | `true` |
30+
| `serviceAccount.name` | Service account name | `hub-agent-sa` |
31+
| `resources` | Resource requests/limits for the container | limits: 500m CPU, 1Gi; requests: 100m CPU, 128Mi |
32+
| `affinity` | Node affinity for hub-agent pods | `{}` |
33+
| `tolerations` | Tolerations for hub-agent pods | `[]` |
34+
| `logVerbosity` | Log level (klog V logs) | `5` |
35+
| `enableV1Alpha1APIs` | Watch for v1alpha1 APIs | `false` |
36+
| `enableV1Beta1APIs` | Watch for v1beta1 APIs | `true` |
37+
| `hubAPIQPS` | QPS for fleet-apiserver (not including events/node heartbeat) | `250` |
38+
| `hubAPIBurst` | Burst for fleet-apiserver (not including events/node heartbeat) | `1000` |
39+
| `MaxConcurrentClusterPlacement` | Max concurrent ClusterResourcePlacement operations | `100` |
40+
| `ConcurrentResourceChangeSyncs` | Max concurrent resourceChange reconcilers | `20` |
41+
| `logFileMaxSize` | Max log file size before rotation | `1000000` |
42+
| `MaxFleetSizeSupported` | Max number of member clusters supported | `100` |
43+
| `resourceSnapshotCreationMinimumInterval` | The minimum interval at which resource snapshots could be created. | `30s` |
44+
| `resourceChangesCollectionDuration` | The duration for collecting resource changes into one snapshot. | `15s` |

charts/hub-agent/templates/deployment.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ spec:
4343
- --hub-api-burst={{ .Values.hubAPIBurst }}
4444
- --force-delete-wait-time={{ .Values.forceDeleteWaitTime }}
4545
- --cluster-unhealthy-threshold={{ .Values.clusterUnhealthyThreshold }}
46-
- --resource-snapshot-creation-interval={{ .Values.resourceSnapshotCreationInterval }}
46+
- --resource-snapshot-creation-minimum-interval={{ .Values.resourceSnapshotCreationMinimumInterval }}
4747
- --resource-changes-collection-duration={{ .Values.resourceChangesCollectionDuration }}
4848
ports:
4949
- name: metrics

charts/hub-agent/values.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ enableGuardRail: true
1818
webhookClientConnectionType: service
1919
forceDeleteWaitTime: 15m0s
2020
clusterUnhealthyThreshold: 3m0s
21-
resourceSnapshotCreationInterval: 30s
22-
resourceChangesCollectionDuration: 30s
21+
resourceSnapshotCreationMinimumInterval: 30s
22+
resourceChangesCollectionDuration: 15s
2323

2424
namespace:
2525
fleet-system

cmd/hubagent/options/options.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,9 @@ type Options struct {
104104
PprofPort int
105105
// DenyModifyMemberClusterLabels indicates if the member cluster labels cannot be modified by groups (excluding system:masters)
106106
DenyModifyMemberClusterLabels bool
107-
// ResourceSnapshotCreationInterval is the interval at which resource snapshots could be created.
108-
// Whether the resource snapshot is created or not depends on the both ResourceSnapshotCreationInterval and resourceChangesCollectionDuration.
109-
ResourceSnapshotCreationInterval time.Duration
107+
// ResourceSnapshotCreationMinimumInterval is the minimum interval at which resource snapshots could be created.
108+
// Whether the resource snapshot is created or not depends on the both ResourceSnapshotCreationMinimumInterval and ResourceChangesCollectionDuration.
109+
ResourceSnapshotCreationMinimumInterval time.Duration
110110
// ResourceChangesCollectionDuration is the duration for collecting resource changes into one snapshot.
111111
ResourceChangesCollectionDuration time.Duration
112112
}
@@ -120,16 +120,16 @@ func NewOptions() *Options {
120120
ResourceNamespace: utils.FleetSystemNamespace,
121121
ResourceName: "136224848560.hub.fleet.azure.com",
122122
},
123-
MaxConcurrentClusterPlacement: 10,
124-
ConcurrentResourceChangeSyncs: 1,
125-
MaxFleetSizeSupported: 100,
126-
EnableV1Alpha1APIs: false,
127-
EnableClusterInventoryAPIs: true,
128-
EnableStagedUpdateRunAPIs: true,
129-
EnablePprof: false,
130-
PprofPort: 6065,
131-
ResourceSnapshotCreationInterval: 30 * time.Second,
132-
ResourceChangesCollectionDuration: 30 * time.Second,
123+
MaxConcurrentClusterPlacement: 10,
124+
ConcurrentResourceChangeSyncs: 1,
125+
MaxFleetSizeSupported: 100,
126+
EnableV1Alpha1APIs: false,
127+
EnableClusterInventoryAPIs: true,
128+
EnableStagedUpdateRunAPIs: true,
129+
EnablePprof: false,
130+
PprofPort: 6065,
131+
ResourceSnapshotCreationMinimumInterval: 30 * time.Second,
132+
ResourceChangesCollectionDuration: 15 * time.Second,
133133
}
134134
}
135135

@@ -176,8 +176,8 @@ func (o *Options) AddFlags(flags *flag.FlagSet) {
176176
flags.BoolVar(&o.EnablePprof, "enable-pprof", false, "If set, the pprof profiling is enabled.")
177177
flags.IntVar(&o.PprofPort, "pprof-port", 6065, "The port for pprof profiling.")
178178
flags.BoolVar(&o.DenyModifyMemberClusterLabels, "deny-modify-member-cluster-labels", false, "If set, users not in the system:masters cannot modify member cluster labels.")
179-
flags.DurationVar(&o.ResourceSnapshotCreationInterval, "resource-snapshot-creation-interval", 30*time.Second, "The interval at which resource snapshots could be created.")
180-
flags.DurationVar(&o.ResourceChangesCollectionDuration, "resource-changes-collection-duration", 30*time.Second,
179+
flags.DurationVar(&o.ResourceSnapshotCreationMinimumInterval, "resource-snapshot-creation-minimum-interval", 30*time.Second, "The minimum interval at which resource snapshots could be created.")
180+
flags.DurationVar(&o.ResourceChangesCollectionDuration, "resource-changes-collection-duration", 15*time.Second,
181181
"The duration for collecting resource changes into one snapshot. The default is 30 seconds, which means that the controller will collect resource changes for 30 seconds before creating a resource snapshot.")
182182
o.RateLimiterOpts.AddFlags(flags)
183183
}

cmd/hubagent/workload/setup.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -153,16 +153,16 @@ func SetupControllers(ctx context.Context, wg *sync.WaitGroup, mgr ctrl.Manager,
153153

154154
// Set up a custom controller to reconcile cluster resource placement
155155
crpc := &clusterresourceplacement.Reconciler{
156-
Client: mgr.GetClient(),
157-
Recorder: mgr.GetEventRecorderFor(crpControllerName),
158-
RestMapper: mgr.GetRESTMapper(),
159-
InformerManager: dynamicInformerManager,
160-
ResourceConfig: resourceConfig,
161-
SkippedNamespaces: skippedNamespaces,
162-
Scheme: mgr.GetScheme(),
163-
UncachedReader: mgr.GetAPIReader(),
164-
ResourceSnapshotCreationInterval: opts.ResourceSnapshotCreationInterval,
165-
ResourceChangesCollectionDuration: opts.ResourceChangesCollectionDuration,
156+
Client: mgr.GetClient(),
157+
Recorder: mgr.GetEventRecorderFor(crpControllerName),
158+
RestMapper: mgr.GetRESTMapper(),
159+
InformerManager: dynamicInformerManager,
160+
ResourceConfig: resourceConfig,
161+
SkippedNamespaces: skippedNamespaces,
162+
Scheme: mgr.GetScheme(),
163+
UncachedReader: mgr.GetAPIReader(),
164+
ResourceSnapshotCreationMinimumInterval: opts.ResourceSnapshotCreationMinimumInterval,
165+
ResourceChangesCollectionDuration: opts.ResourceChangesCollectionDuration,
166166
}
167167

168168
rateLimiter := options.DefaultControllerRateLimiter(opts.RateLimiterOpts)

pkg/controllers/clusterresourceplacement/controller.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,9 @@ func (r *Reconciler) getOrCreateClusterResourceSnapshot(ctx context.Context, crp
585585
}
586586

587587
// shouldCreateNewResourceSnapshotNow checks whether it is ready to create the new resource snapshot to avoid too frequent creation
588-
// based on the configured ResourceSnapshotCreationInterval.
588+
// based on the configured resourceSnapshotCreationMinimumInterval and resourceChangesCollectionDuration.
589589
func (r *Reconciler) shouldCreateNewResourceSnapshotNow(ctx context.Context, latestResourceSnapshot fleetv1beta1.ResourceSnapshotObj) (ctrl.Result, error) {
590-
if r.ResourceSnapshotCreationInterval <= 0 && r.ResourceChangesCollectionDuration <= 0 {
590+
if r.ResourceSnapshotCreationMinimumInterval <= 0 && r.ResourceChangesCollectionDuration <= 0 {
591591
return ctrl.Result{}, nil
592592
}
593593

@@ -611,12 +611,12 @@ func (r *Reconciler) shouldCreateNewResourceSnapshotNow(ctx context.Context, lat
611611
nextResourceSnapshotCandidateDetectionTime = now
612612
klog.V(2).InfoS("Updated the NextResourceSnapshotCandidateDetectionTime annotation", "clusterResourceSnapshot", snapshotKObj, "nextResourceSnapshotCandidateDetectionTimeAnnotation", now.Format(time.RFC3339))
613613
}
614-
nextCreationTime := fleettime.MaxTime(nextResourceSnapshotCandidateDetectionTime.Add(r.ResourceChangesCollectionDuration), latestResourceSnapshot.GetCreationTimestamp().Add(r.ResourceSnapshotCreationInterval))
614+
nextCreationTime := fleettime.MaxTime(nextResourceSnapshotCandidateDetectionTime.Add(r.ResourceChangesCollectionDuration), latestResourceSnapshot.GetCreationTimestamp().Add(r.ResourceSnapshotCreationMinimumInterval))
615615
if now.Before(nextCreationTime) {
616616
// If the next resource snapshot creation time is not reached, we requeue the request to avoid too frequent update.
617617
klog.V(2).InfoS("Delaying the new resourceSnapshot creation",
618618
"clusterResourceSnapshot", snapshotKObj, "nextCreationTime", nextCreationTime, "latestResourceSnapshotCreationTime", latestResourceSnapshot.GetCreationTimestamp(),
619-
"resourceSnapshotCreationInterval", r.ResourceSnapshotCreationInterval, "resourceChangesCollectionDuration", r.ResourceChangesCollectionDuration,
619+
"resourceSnapshotCreationMinimumInterval", r.ResourceSnapshotCreationMinimumInterval, "resourceChangesCollectionDuration", r.ResourceChangesCollectionDuration,
620620
"afterDuration", nextCreationTime.Sub(now))
621621
return ctrl.Result{Requeue: true, RequeueAfter: nextCreationTime.Sub(now)}, nil
622622
}

0 commit comments

Comments
 (0)