-
Notifications
You must be signed in to change notification settings - Fork 428
Description
Describe the bug
The validating admission webhook for Tiers checks for overlapping priorities, and returns an error if the priority of a new Tier matches the priority of an existing Tier:
antrea/pkg/controller/networkpolicy/validate.go
Lines 992 to 1008 in ea0bb2c
| // createValidate validates the CREATE events of Tier resources. | |
| func (t *tierValidator) createValidate(curObj interface{}, userInfo authenticationv1.UserInfo) ([]string, string, bool) { | |
| if len(t.networkPolicyController.tierInformer.Informer().GetIndexer().ListIndexFuncValues(PriorityIndex)) >= maxSupportedTiers { | |
| return nil, fmt.Sprintf("maximum number of Tiers supported: %d", maxSupportedTiers), false | |
| } | |
| curTier := curObj.(*crdv1beta1.Tier) | |
| // Tier priority must not overlap reserved tier's priority. | |
| if reservedTierPriorities.Has(curTier.Spec.Priority) { | |
| return nil, fmt.Sprintf("tier %s priority %d is reserved", curTier.Name, curTier.Spec.Priority), false | |
| } | |
| // Tier priority must not overlap existing tier's priority | |
| trs, err := t.networkPolicyController.tierInformer.Informer().GetIndexer().ByIndex(PriorityIndex, strconv.FormatInt(int64(curTier.Spec.Priority), 10)) | |
| if err != nil || len(trs) > 0 { | |
| return nil, fmt.Sprintf("tier %s priority %d overlaps with existing Tier", curTier.Name, curTier.Spec.Priority), false | |
| } | |
| return nil, "", true | |
| } |
However, it is possible to "time" Tier creation so that 2 Tiers with the same priority can be created at the same time.
To Reproduce
I was able to create 2 Tiers with the same priority using this script:
kubectl apply -f tier1.yml&
kubectl apply -f tier2.yml&
sleep 1
kubectl get tiers -o wide$ ./create_tiers.sh
tier.crd.antrea.io/mytier2 created
tier.crd.antrea.io/mytier1 created
NAME PRIORITY AGE
application 250 3m59s
baseline 253 3m59s
emergency 50 3m59s
mytier1 10 1s
mytier2 10 1s
networkops 150 3m59s
platform 200 3m59s
securityops 100 3m59stier1.yml
apiVersion: crd.antrea.io/v1beta1
kind: Tier
metadata:
name: mytier1
spec:
priority: 10tier2.yml
apiVersion: crd.antrea.io/v1beta1
kind: Tier
metadata:
name: mytier2
spec:
priority: 10Expected
Creation of one of the Tiers (either mytier1 or mytier2) should fail.
Actual behavior
Both Tiers are created succesfully.
Versions:
Antrea v2.4.0
Additional context
The validating webhook uses the informer directly to keep track of existing Tiers and their priorities (with the appropriate index). While it is simple, it means that a race is possible. For example:
- validating webhook is invoked for
mytier1, success mytier1is created- validating webhook is invoked for
mytier2, success - watchers are notified and the antrea-controller's informer is updated with
mytier1 mytier2is created- watchers are notified and the antrea-controller's informer is updated with
mytier2
A "correct" implementation of the validating webhook would handle all requests sequentially and keep track of all priorities currently in use. However, this would increase the complexity of the implementation. As per the K8s documentation:
If a webhook called by this has side effects (for example, decrementing quota) it must have a reconciliation system, as it is not guaranteed that subsequent webhooks or other validating admission controllers will permit the request to finish.
The webhook would need to frequently reconcile its internal thread-safe priority "set" with the actual API resources.