fleet: init fleet webhook and ensure single fleet per namespace #409

Xieql · 2023-10-23T03:22:45Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This PR introduces validation for the Fleet custom resource using webhooks. The primary changes include:

Fleet Validation:
- Added checks to ensure that there's only one Fleet instance in a namespace. see Limit Fleet CRD to One Instance per Namespace #382
- The kind field of a Cluster can only take the values Cluster, AttachedCluster, or CustomCluster. Any other values are considered invalid.
Unit Tests:
Added unit tests to validate the logic in the Fleet webhook.
Created sample yaml files for both valid and invalid Fleet configurations to aid testing.
Here is the result of UT:

=== RUN   TestValidFleetValidation
--- PASS: TestValidFleetValidation (0.00s)
=== RUN   TestValidFleetValidation/..\..\examples\fleet\fleet.yaml
    --- PASS: TestValidFleetValidation/..\..\examples\fleet\fleet.yaml (0.00s)
=== RUN   TestValidFleetValidation/..\..\examples\fleet\metric\metric-plugin.yaml
    --- PASS: TestValidFleetValidation/..\..\examples\fleet\metric\metric-plugin.yaml (0.00s)
=== RUN   TestValidFleetValidation/..\..\examples\fleet\metric\monitor-demo\avalanche.yaml
    --- PASS: TestValidFleetValidation/..\..\examples\fleet\metric\monitor-demo\avalanche.yaml (0.00s)
=== RUN   TestValidFleetValidation/..\..\examples\fleet\policy\badpod-demo\badpod.yaml
    --- PASS: TestValidFleetValidation/..\..\examples\fleet\policy\badpod-demo\badpod.yaml (0.00s)
=== RUN   TestValidFleetValidation/..\..\examples\fleet\policy\kyverno.yaml
    --- PASS: TestValidFleetValidation/..\..\examples\fleet\policy\kyverno.yaml (0.00s)
=== RUN   TestValidFleetValidation/..\..\examples\fleet\quickstart.yaml
    --- PASS: TestValidFleetValidation/..\..\examples\fleet\quickstart.yaml (0.00s)
=== RUN   TestInvalidFleetValidation
--- PASS: TestInvalidFleetValidation (0.00s)
=== RUN   TestInvalidFleetValidation/testdata\fleet\invalid-cluster-kind.yaml
    fleet_webhook_test.go:47: Fleet.fleet.kurator.dev "quickstart" is invalid: spec.clusters[1].kind: Invalid value: "invalid-cluster-kind": unsupported cluster kind; please use AttachedCluster to manage your own cluster
    --- PASS: TestInvalidFleetValidation/testdata\fleet\invalid-cluster-kind.yaml (0.00s)
=== RUN   TestInvalidFleetValidation/testdata\fleet\miss-cluster-name-kind.yaml
    fleet_webhook_test.go:47: Fleet.fleet.kurator.dev "quickstart" is invalid: [spec.clusters[0].name: Required value: name is required, spec.clusters[1].kind: Required value: kind is required]
    --- PASS: TestInvalidFleetValidation/testdata\fleet\miss-cluster-name-kind.yaml (0.00s)
PASS

Which issue(s) this PR fixes:

part of #404
part of #336
Fixes #382

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

fleet: init fleet webhook and ensure single fleet per namespace

netlify · 2023-10-23T03:22:49Z

✅ Deploy Preview for kurator-dev canceled.

Name	Link
🔨 Latest commit	`d6f7e99`
🔍 Latest deploy log	https://app.netlify.com/sites/kurator-dev/deploys/65361d38b5c1220008b5862e

kurator-bot · 2023-10-23T03:22:51Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kevin-wangzefeng for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Xieql <[email protected]>

hzxuzhonghu · 2023-10-23T07:44:51Z

pkg/webhooks/fleet_webhook.go

+	return nsLocks[ns]
+}
+
+func (wh *FleetWebhook) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) error {


For update, we should prebent some fields update we do not support, this need to be reviewed one by one

hzxuzhonghu · 2023-10-23T07:46:49Z

pkg/webhooks/fleet_webhook.go

+
+	// Check if Fleet instance already exists in the namespace
+	existing := &v1alpha1.Fleet{}
+	if err := wh.Client.Get(ctx, client.ObjectKey{Namespace: in.Namespace, Name: in.Name}, existing); err == nil {


not sure i understand

k8s has prevented creating same fleet

hzxuzhonghu · 2023-10-23T07:49:38Z

pkg/webhooks/fleet_webhook.go

+
+// Utility function to get or create a mutex for a namespace
+func getOrCreateMutexForNamespace(ns string) *sync.Mutex {
+	if _, exists := nsLocks[ns]; !exists {


data race r/w nsLocks

hzxuzhonghu · 2023-10-23T07:51:54Z

pkg/webhooks/fleet_webhook.go

+
+	// Ensure only one Fleet instance in a namespace
+	mutex := getOrCreateMutexForNamespace(in.Namespace)
+	mutex.Lock()


The lock can only prevent inner-process creating, it cannot help for multi replicate scenarios.

One way as I said in the original issue, could make use of distributed lock.

hzxuzhonghu · 2023-10-26T01:15:35Z

maybe we can use resource quota, install fleet manager with a resourceQuota

Xieql · 2023-10-26T01:16:41Z

maybe we can use resource quota, install fleet manager with a resourceQuota

ok

Xieql · 2023-11-24T01:53:45Z

Given that the current design, along with the resourceQuota settings, are inadequate in addressing the issue of ensuring a single fleet per namespace (as detailed in Issue #382), I am closing this pull request.

kurator-bot added the kind/feature label Oct 23, 2023

kurator-bot requested review from hzxuzhonghu and kevin-wangzefeng October 23, 2023 03:22

kurator-bot added the size/L label Oct 23, 2023

Xieql force-pushed the init-fleet-webhook branch from 2e3c81c to 300453a Compare October 23, 2023 03:31

Xieql mentioned this pull request Oct 23, 2023

[FR] Unified Backup, Restore, and Migration #404

Closed

19 tasks

Xieql force-pushed the init-fleet-webhook branch from 300453a to 10fbd6b Compare October 23, 2023 06:26

fleet: init fleet webhook

d6f7e99

Signed-off-by: Xieql <[email protected]>

Xieql force-pushed the init-fleet-webhook branch from 10fbd6b to d6f7e99 Compare October 23, 2023 07:13

hzxuzhonghu reviewed Oct 23, 2023

View reviewed changes

Xieql closed this Nov 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fleet: init fleet webhook and ensure single fleet per namespace #409

fleet: init fleet webhook and ensure single fleet per namespace #409

Xieql commented Oct 23, 2023 •

edited

Loading

netlify bot commented Oct 23, 2023 •

edited

Loading

kurator-bot commented Oct 23, 2023

hzxuzhonghu Oct 23, 2023

hzxuzhonghu Oct 23, 2023

hzxuzhonghu Oct 23, 2023

hzxuzhonghu Oct 23, 2023

hzxuzhonghu Oct 23, 2023

hzxuzhonghu commented Oct 26, 2023

Xieql commented Oct 26, 2023

Xieql commented Nov 24, 2023 •

edited

Loading

fleet: init fleet webhook and ensure single fleet per namespace #409

fleet: init fleet webhook and ensure single fleet per namespace #409

Conversation

Xieql commented Oct 23, 2023 • edited Loading

netlify bot commented Oct 23, 2023 • edited Loading

✅ Deploy Preview for kurator-dev canceled.

kurator-bot commented Oct 23, 2023

hzxuzhonghu Oct 23, 2023

Choose a reason for hiding this comment

hzxuzhonghu Oct 23, 2023

Choose a reason for hiding this comment

hzxuzhonghu Oct 23, 2023

Choose a reason for hiding this comment

hzxuzhonghu Oct 23, 2023

Choose a reason for hiding this comment

hzxuzhonghu Oct 23, 2023

Choose a reason for hiding this comment

hzxuzhonghu commented Oct 26, 2023

Xieql commented Oct 26, 2023

Xieql commented Nov 24, 2023 • edited Loading

Xieql commented Oct 23, 2023 •

edited

Loading

netlify bot commented Oct 23, 2023 •

edited

Loading

Xieql commented Nov 24, 2023 •

edited

Loading