From 03e2befde50b6c43c1b9599bbe680e93078fc9eb Mon Sep 17 00:00:00 2001 From: Moshe Immerman Date: Sun, 2 May 2021 10:35:10 +0000 Subject: [PATCH 1/4] feat: update controller-runtime --- config/deploy/base.yml | 3 +- config/operator/certmanager/certificate.yaml | 10 +- config/operator/default/kustomization.yaml | 7 +- config/operator/default/namespace.yaml | 2 +- .../default/webhookcainjection_patch.yaml | 4 +- config/operator/rbac/role.yaml | 3 +- config/operator/webhook/manifests.yaml | 160 ++++++++++-------- go.mod | 4 +- go.sum | 2 + 9 files changed, 102 insertions(+), 93 deletions(-) diff --git a/config/deploy/base.yml b/config/deploy/base.yml index 5e8ae37..9d88476 100644 --- a/config/deploy/base.yml +++ b/config/deploy/base.yml @@ -106,10 +106,8 @@ rules: resources: - namespaces verbs: - - delete - get - list - - watch - apiGroups: - "" resources: @@ -146,6 +144,7 @@ rules: - update - apiGroups: - extensions + - networking.k8s.io resources: - ingresses verbs: diff --git a/config/operator/certmanager/certificate.yaml b/config/operator/certmanager/certificate.yaml index 70fc6f1..f039262 100644 --- a/config/operator/certmanager/certificate.yaml +++ b/config/operator/certmanager/certificate.yaml @@ -1,7 +1,7 @@ # The following manifests contain a self-signed issuer CR and a certificate CR. # More document can be found at https://docs.cert-manager.io # WARNING: Targets CertManager 0.11 check https://docs.cert-manager.io/en/latest/tasks/upgrading/index.html for breaking changes -apiVersion: cert-manager.io/v1alpha2 +apiVersion: cert-manager.io/v1 kind: Issuer metadata: name: selfsigned-issuer @@ -9,16 +9,16 @@ metadata: spec: selfSigned: {} --- -apiVersion: cert-manager.io/v1alpha2 +apiVersion: cert-manager.io/v1 kind: Certificate metadata: - name: serving-cert # this name should match the one appeared in kustomizeconfig.yaml + name: serving-cert # this name should match the one appeared in kustomizeconfig.yaml namespace: system spec: # $(SERVICE_NAME) and $(SERVICE_NAMESPACE) will be substituted by kustomize dnsNames: - - $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc - - $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc.cluster.local + - $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc + - $(SERVICE_NAME).$(SERVICE_NAMESPACE).svc.cluster.local issuerRef: kind: Issuer name: selfsigned-issuer diff --git a/config/operator/default/kustomization.yaml b/config/operator/default/kustomization.yaml index b36b075..a4c546e 100644 --- a/config/operator/default/kustomization.yaml +++ b/config/operator/default/kustomization.yaml @@ -20,7 +20,6 @@ patchesStrategicMerge: - webhookcainjection_patch.yaml # +kubebuilder:scaffold:crdkustomizecainjectionpatch - bases: - ../../crds - ../rbac @@ -34,7 +33,7 @@ vars: objref: kind: Certificate group: cert-manager.io - version: v1alpha2 + version: v1 name: serving-cert # this name should match the one in certificate.yaml fieldref: fieldpath: metadata.namespace @@ -42,7 +41,7 @@ vars: objref: kind: Certificate group: cert-manager.io - version: v1alpha2 + version: v1 name: serving-cert # this name should match the one in certificate.yaml - name: SERVICE_NAMESPACE # namespace of the service objref: @@ -55,4 +54,4 @@ vars: objref: kind: Service version: v1 - name: operator \ No newline at end of file + name: operator diff --git a/config/operator/default/namespace.yaml b/config/operator/default/namespace.yaml index 044774d..294ca4b 100644 --- a/config/operator/default/namespace.yaml +++ b/config/operator/default/namespace.yaml @@ -3,4 +3,4 @@ kind: Namespace metadata: labels: control-plane: platform-operator - name: platform-system \ No newline at end of file + name: system diff --git a/config/operator/default/webhookcainjection_patch.yaml b/config/operator/default/webhookcainjection_patch.yaml index 965a5ef..d2b6f87 100644 --- a/config/operator/default/webhookcainjection_patch.yaml +++ b/config/operator/default/webhookcainjection_patch.yaml @@ -1,12 +1,12 @@ --- -apiVersion: admissionregistration.k8s.io/v1beta1 +apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: name: validating-webhook-configuration annotations: cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) --- -apiVersion: admissionregistration.k8s.io/v1beta1 +apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: name: mutating-webhook-configuration diff --git a/config/operator/rbac/role.yaml b/config/operator/rbac/role.yaml index 72c2971..c675eb3 100644 --- a/config/operator/rbac/role.yaml +++ b/config/operator/rbac/role.yaml @@ -11,10 +11,8 @@ rules: resources: - namespaces verbs: - - delete - get - list - - watch - apiGroups: - "" resources: @@ -51,6 +49,7 @@ rules: - update - apiGroups: - extensions + - networking.k8s.io resources: - ingresses verbs: diff --git a/config/operator/webhook/manifests.yaml b/config/operator/webhook/manifests.yaml index f13bc1f..ed45324 100644 --- a/config/operator/webhook/manifests.yaml +++ b/config/operator/webhook/manifests.yaml @@ -1,88 +1,98 @@ - --- -apiVersion: admissionregistration.k8s.io/v1beta1 +apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: creationTimestamp: null name: mutating-webhook-configuration webhooks: -- clientConfig: - caBundle: Cg== - service: - name: operator - namespace: system - path: /mutate-v1-ingress - failurePolicy: Ignore - name: annotate-ingress-v1.platform.flanksource.com - rules: - - apiGroups: - - extensions - apiVersions: - - v1beta1 - operations: - - CREATE - - UPDATE - resources: - - ingresses -- clientConfig: - caBundle: Cg== - service: - name: operator - namespace: system - path: /mutate-v1-pod - failurePolicy: Ignore - name: annotate-pods-v1.platform.flanksource.com - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - pods + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-v1-pod + failurePolicy: Ignore + name: mutate-v1-pod.platform.flanksource.com + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - pods + sideEffects: None + + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-v1-ingress + failurePolicy: Ignore + name: mutate-v1-pod.platform.flanksource.com + rules: + - apiGroups: + - extensions + - networking.k8s.io + apiVersions: + - v1beta1 + - v1 + operations: + - CREATE + - UPDATE + resources: + - ingresses + sideEffects: None --- -apiVersion: admissionregistration.k8s.io/v1beta1 +apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: creationTimestamp: null name: validating-webhook-configuration webhooks: -- clientConfig: - caBundle: Cg== - service: - name: operator - namespace: system - path: /validate-clusterresourcequota-platform-flanksource-com-v1 - failurePolicy: Fail - name: clusterresourcequotas-validation-v1.platform.flanksource.com - rules: - - apiGroups: - - platform.flanksource.com - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - clusterresourcequotas -- clientConfig: - caBundle: Cg== - service: - name: operator - namespace: system - path: /validate-resourcequota-v1 - failurePolicy: Fail - name: resourcequotas-validation-v1.platform.flanksource.com - rules: - - apiGroups: - - "" - apiVersions: - - v1 - operations: - - CREATE - - UPDATE - resources: - - resourcequotas + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-clusterresourcequota-platform-flanksource-com-v1 + failurePolicy: Fail + name: clusterresourcequotas-validation-v1.platform.flanksource.com + rules: + - apiGroups: + - platform.flanksource.com + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - clusterresourcequotas + sideEffects: None + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-resourcequota-platform-flanksource-com-v1 + failurePolicy: Fail + name: resourcequotas-validation-v1.platform.flanksource.com + rules: + - apiGroups: + - "" + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - resourcequotas + sideEffects: None diff --git a/go.mod b/go.mod index 6bbb85b..2810c20 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( k8s.io/client-go v11.0.0+incompatible k8s.io/klog v1.0.0 k8s.io/utils v0.0.0-20210111153108-fddb29f9d009 - sigs.k8s.io/controller-runtime v0.8.2 + sigs.k8s.io/controller-runtime v0.8.3 sigs.k8s.io/controller-tools v0.5.0 sigs.k8s.io/yaml v1.2.0 ) @@ -36,7 +36,7 @@ replace ( k8s.io/apimachinery => k8s.io/apimachinery v0.20.4 k8s.io/apiserver => k8s.io/apiserver v0.20.4 k8s.io/client-go => k8s.io/client-go v0.20.4 - sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.8.2 + sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.8.3 launchpad.net/gocheck => github.com/go-check/check v0.0.0-20180628173108-788fd7840127 vbom.ml/util => github.com/fvbommel/util v0.0.0-20180919145318-efcd4e0f9787 ) diff --git a/go.sum b/go.sum index c36b6e9..5c7f483 100644 --- a/go.sum +++ b/go.sum @@ -1711,6 +1711,8 @@ sigs.k8s.io/controller-runtime v0.4.0 h1:wATM6/m+3w8lj8FXNaO6Fs/rq/vqoOjO1Q116Z9 sigs.k8s.io/controller-runtime v0.4.0/go.mod h1:ApC79lpY3PHW9xj/w9pj+lYkLgwAAUZwfXkME1Lajns= sigs.k8s.io/controller-runtime v0.8.2 h1:SBWmI0b3uzMIUD/BIXWNegrCeZmPJ503pOtwxY0LPHM= sigs.k8s.io/controller-runtime v0.8.2/go.mod h1:U/l+DUopBc1ecfRZ5aviA9JDmGFQKvLf5YkZNx2e0sU= +sigs.k8s.io/controller-runtime v0.8.3 h1:GMHvzjTmaWHQB8HadW+dIvBoJuLvZObYJ5YoZruPRao= +sigs.k8s.io/controller-runtime v0.8.3/go.mod h1:U/l+DUopBc1ecfRZ5aviA9JDmGFQKvLf5YkZNx2e0sU= sigs.k8s.io/controller-tools v0.5.0 h1:3u2RCwOlp0cjCALAigpOcbAf50pE+kHSdueUosrC/AE= sigs.k8s.io/controller-tools v0.5.0/go.mod h1:JTsstrMpxs+9BUj6eGuAaEb6SDSPTeVtUyp0jmnAM/I= sigs.k8s.io/kustomize v2.0.3+incompatible/go.mod h1:MkjgH3RdOWrievjo6c9T245dYlB5QeXV4WCbnt/PEpU= From c736770a27f9002e2c5dbdc0896b025dc9165987 Mon Sep 17 00:00:00 2001 From: Moshe Immerman Date: Sun, 2 May 2021 10:36:31 +0000 Subject: [PATCH 2/4] fix: clusterresourcequotas --- Makefile | 2 +- cmd/manager/main.go | 2 +- ...flanksource.com_clusterresourcequotas.yaml | 140 +++++-------- config/deploy/crd.yml | 121 ++++++----- config/deploy/manifests.yaml | 194 +++++++++--------- .../platform/v1/clusterresourcequota_types.go | 9 +- pkg/apis/platform/v1/zz_generated.deepcopy.go | 9 +- pkg/controllers/cleanup/cleanup_controller.go | 1 - .../clusterresourcequota_controller.go | 84 +++----- .../clusterresourcequota_controller_test.go | 2 +- .../clusterresourcequota_validatingwebhook.go | 41 +--- .../resourcequota_validatingwebhook.go | 62 ++---- .../clusterresourcequota/resources.go | 114 ++++++++++ pkg/controllers/clusterresourcequota/utils.go | 40 ++++ pkg/controllers/ingress/ingress_reconciler.go | 2 +- pkg/controllers/ingress/mutating_webhook.go | 2 +- pkg/controllers/pod/mutating_webhook.go | 2 +- test/clusterresourcequota_test.go | 168 ++++++++------- test/suite_test.go | 23 ++- 19 files changed, 557 insertions(+), 461 deletions(-) create mode 100644 pkg/controllers/clusterresourcequota/utils.go diff --git a/Makefile b/Makefile index b786a8f..19515d6 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ endif # download controller-gen if necessary controller-gen: ifeq ($(shell command -v controller-gen),) - @(cd /tmp; GO111MODULE=on go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.2.4) + @(cd /tmp; GO111MODULE=on go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.5.0) CONTROLLER_GEN=$(GOBIN)/controller-gen else CONTROLLER_GEN=$(shell which controller-gen) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 40dfffd..e85c01b 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -127,7 +127,7 @@ func main() { setupLog.Error(err, "unable to create controller", "controller", "ClusterResourceQuota") os.Exit(1) } - hookServer.Register("/validate-clusterresourcequota-platform-flanksource-com-v1", clusterresourcequota.NewClusterResourceQuotaValidatingWebhook(mgr.GetClient(), mtx, enableClusterResourceQuota)) + hookServer.Register("/validate-clusterresourcequota-v1", clusterresourcequota.NewClusterResourceQuotaValidatingWebhook(mgr.GetClient(), mtx, enableClusterResourceQuota)) hookServer.Register("/validate-resourcequota-v1", clusterresourcequota.NewResourceQuotaValidatingWebhook(mgr.GetClient(), mtx, enableClusterResourceQuota)) } diff --git a/config/crds/bases/platform.flanksource.com_clusterresourcequotas.yaml b/config/crds/bases/platform.flanksource.com_clusterresourcequotas.yaml index c6178e5..bc4e3e0 100644 --- a/config/crds/bases/platform.flanksource.com_clusterresourcequotas.yaml +++ b/config/crds/bases/platform.flanksource.com_clusterresourcequotas.yaml @@ -4,7 +4,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.4.0 + controller-gen.kubebuilder.io/version: v0.5.0 creationTimestamp: null name: clusterresourcequotas.platform.flanksource.com spec: @@ -19,104 +19,79 @@ spec: - name: v1 schema: openAPIV3Schema: - description: ClusterResourceQuota is the Schema for the clusterresourcequotas - API + description: ClusterResourceQuota is the Schema for the clusterresourcequotas API properties: apiVersion: - description: 'APIVersion defines the versioned schema of this representation - of an object. Servers should convert recognized schemas to the latest - internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' type: string kind: - description: 'Kind is a string value representing the REST resource this - object represents. Servers may infer this from the endpoint the client - submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' type: string metadata: type: object spec: description: Spec defines the desired quota properties: - quota: - description: Quota sets aggregate quota restrictions enforced across - all namespaces + hard: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'hard is the set of desired hard limits for each named resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' + type: object + matchLabels: + additionalProperties: + type: string + type: object + scopeSelector: + description: scopeSelector is also a collection of filters like scopes that must match each object tracked by a quota but expressed using ScopeSelectorOperator in combination with possible values. For a resource to match, both scopes AND scopeSelector (if specified in spec), must be matched. properties: - hard: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'hard is the set of desired hard limits for each - named resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' - type: object - scopeSelector: - description: scopeSelector is also a collection of filters like - scopes that must match each object tracked by a quota but expressed - using ScopeSelectorOperator in combination with possible values. - For a resource to match, both scopes AND scopeSelector (if specified - in spec), must be matched. - properties: - matchExpressions: - description: A list of scope selector requirements by scope - of the resources. - items: - description: A scoped-resource selector requirement is a - selector that contains values, a scope name, and an operator - that relates the scope name and values. - properties: - operator: - description: Represents a scope's relationship to a - set of values. Valid operators are In, NotIn, Exists, - DoesNotExist. - type: string - scopeName: - description: The name of the scope that the selector - applies to. - type: string - values: - description: An array of string values. If the operator - is In or NotIn, the values array must be non-empty. - If the operator is Exists or DoesNotExist, the values - array must be empty. This array is replaced during - a strategic merge patch. - items: - type: string - type: array - required: - - operator - - scopeName - type: object - type: array - type: object - scopes: - description: A collection of filters that must match each object - tracked by a quota. If not specified, the quota matches all - objects. + matchExpressions: + description: A list of scope selector requirements by scope of the resources. items: - description: A ResourceQuotaScope defines a filter that must - match each object tracked by a quota - type: string + description: A scoped-resource selector requirement is a selector that contains values, a scope name, and an operator that relates the scope name and values. + properties: + operator: + description: Represents a scope's relationship to a set of values. Valid operators are In, NotIn, Exists, DoesNotExist. + type: string + scopeName: + description: The name of the scope that the selector applies to. + type: string + values: + description: An array of string values. If the operator is In or NotIn, the values array must be non-empty. If the operator is Exists or DoesNotExist, the values array must be empty. This array is replaced during a strategic merge patch. + items: + type: string + type: array + required: + - operator + - scopeName + type: object type: array type: object + scopes: + description: A collection of filters that must match each object tracked by a quota. If not specified, the quota matches all objects. + items: + description: A ResourceQuotaScope defines a filter that must match each object tracked by a quota + type: string + type: array + required: + - matchLabels type: object status: - description: Status defines the actual enforced quota and its current - usage + description: Status defines the actual enforced quota and its current usage properties: namespaces: description: Slices the quota used per namespace items: - description: ResourceQuotaStatusByNamespace gives status for a particular - name + description: ResourceQuotaStatusByNamespace gives status for a particular name properties: namespace: description: Namespace the project this status applies to type: string status: - description: Status indicates how many resources have been consumed - by this project + description: Status indicates how many resources have been consumed by this project properties: hard: additionalProperties: @@ -125,8 +100,7 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: 'Hard is the set of enforced hard limits for - each named resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' + description: 'Hard is the set of enforced hard limits for each named resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' type: object used: additionalProperties: @@ -135,8 +109,7 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: Used is the current observed total usage of - the resource in the namespace. + description: Used is the current observed total usage of the resource in the namespace. type: object type: object required: @@ -145,8 +118,7 @@ spec: type: object type: array total: - description: Total defines the actual enforced quota and its current - usage across all namespaces + description: Total defines the actual enforced quota and its current usage across all namespaces properties: hard: additionalProperties: @@ -155,8 +127,7 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: 'Hard is the set of enforced hard limits for each - named resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' + description: 'Hard is the set of enforced hard limits for each named resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' type: object used: additionalProperties: @@ -165,12 +136,9 @@ spec: - type: string pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ x-kubernetes-int-or-string: true - description: Used is the current observed total usage of the resource - in the namespace. + description: Used is the current observed total usage of the resource in the namespace. type: object type: object - required: - - namespaces type: object type: object served: true diff --git a/config/deploy/crd.yml b/config/deploy/crd.yml index 205f7da..bf45299 100644 --- a/config/deploy/crd.yml +++ b/config/deploy/crd.yml @@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - controller-gen.kubebuilder.io/version: v0.4.0 + controller-gen.kubebuilder.io/version: v0.5.0 creationTimestamp: null name: clusterresourcequotas.platform.flanksource.com spec: @@ -35,69 +35,68 @@ spec: spec: description: Spec defines the desired quota properties: - quota: - description: Quota sets aggregate quota restrictions enforced across - all namespaces + hard: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'hard is the set of desired hard limits for each named + resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' + type: object + matchLabels: + additionalProperties: + type: string + type: object + scopeSelector: + description: scopeSelector is also a collection of filters like scopes + that must match each object tracked by a quota but expressed using + ScopeSelectorOperator in combination with possible values. For a + resource to match, both scopes AND scopeSelector (if specified in + spec), must be matched. properties: - hard: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'hard is the set of desired hard limits for each - named resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' - type: object - scopeSelector: - description: scopeSelector is also a collection of filters like - scopes that must match each object tracked by a quota but expressed - using ScopeSelectorOperator in combination with possible values. - For a resource to match, both scopes AND scopeSelector (if specified - in spec), must be matched. - properties: - matchExpressions: - description: A list of scope selector requirements by scope - of the resources. - items: - description: A scoped-resource selector requirement is a - selector that contains values, a scope name, and an operator - that relates the scope name and values. - properties: - operator: - description: Represents a scope's relationship to a - set of values. Valid operators are In, NotIn, Exists, - DoesNotExist. - type: string - scopeName: - description: The name of the scope that the selector - applies to. - type: string - values: - description: An array of string values. If the operator - is In or NotIn, the values array must be non-empty. - If the operator is Exists or DoesNotExist, the values - array must be empty. This array is replaced during - a strategic merge patch. - items: - type: string - type: array - required: - - operator - - scopeName - type: object - type: array - type: object - scopes: - description: A collection of filters that must match each object - tracked by a quota. If not specified, the quota matches all - objects. + matchExpressions: + description: A list of scope selector requirements by scope of + the resources. items: - description: A ResourceQuotaScope defines a filter that must - match each object tracked by a quota - type: string + description: A scoped-resource selector requirement is a selector + that contains values, a scope name, and an operator that relates + the scope name and values. + properties: + operator: + description: Represents a scope's relationship to a set + of values. Valid operators are In, NotIn, Exists, DoesNotExist. + type: string + scopeName: + description: The name of the scope that the selector applies + to. + type: string + values: + description: An array of string values. If the operator + is In or NotIn, the values array must be non-empty. If + the operator is Exists or DoesNotExist, the values array + must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - operator + - scopeName + type: object type: array type: object + scopes: + description: A collection of filters that must match each object tracked + by a quota. If not specified, the quota matches all objects. + items: + description: A ResourceQuotaScope defines a filter that must match + each object tracked by a quota + type: string + type: array + required: + - matchLabels type: object status: description: Status defines the actual enforced quota and its current @@ -167,8 +166,6 @@ spec: in the namespace. type: object type: object - required: - - namespaces type: object type: object served: true diff --git a/config/deploy/manifests.yaml b/config/deploy/manifests.yaml index 6b40c43..381d2fd 100644 --- a/config/deploy/manifests.yaml +++ b/config/deploy/manifests.yaml @@ -3,14 +3,14 @@ kind: Namespace metadata: labels: control-plane: platform-operator - name: platform-platform-system + name: platform-system --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: - cert-manager.io/inject-ca-from: platform-system/platform-serving-cert - controller-gen.kubebuilder.io/version: v0.4.0 + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) + controller-gen.kubebuilder.io/version: v0.5.0 creationTimestamp: null name: clusterresourcequotas.platform.flanksource.com spec: @@ -43,69 +43,68 @@ spec: spec: description: Spec defines the desired quota properties: - quota: - description: Quota sets aggregate quota restrictions enforced across - all namespaces + hard: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: 'hard is the set of desired hard limits for each named + resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' + type: object + matchLabels: + additionalProperties: + type: string + type: object + scopeSelector: + description: scopeSelector is also a collection of filters like scopes + that must match each object tracked by a quota but expressed using + ScopeSelectorOperator in combination with possible values. For a + resource to match, both scopes AND scopeSelector (if specified in + spec), must be matched. properties: - hard: - additionalProperties: - anyOf: - - type: integer - - type: string - pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ - x-kubernetes-int-or-string: true - description: 'hard is the set of desired hard limits for each - named resource. More info: https://kubernetes.io/docs/concepts/policy/resource-quotas/' - type: object - scopeSelector: - description: scopeSelector is also a collection of filters like - scopes that must match each object tracked by a quota but expressed - using ScopeSelectorOperator in combination with possible values. - For a resource to match, both scopes AND scopeSelector (if specified - in spec), must be matched. - properties: - matchExpressions: - description: A list of scope selector requirements by scope - of the resources. - items: - description: A scoped-resource selector requirement is a - selector that contains values, a scope name, and an operator - that relates the scope name and values. - properties: - operator: - description: Represents a scope's relationship to a - set of values. Valid operators are In, NotIn, Exists, - DoesNotExist. - type: string - scopeName: - description: The name of the scope that the selector - applies to. - type: string - values: - description: An array of string values. If the operator - is In or NotIn, the values array must be non-empty. - If the operator is Exists or DoesNotExist, the values - array must be empty. This array is replaced during - a strategic merge patch. - items: - type: string - type: array - required: - - operator - - scopeName - type: object - type: array - type: object - scopes: - description: A collection of filters that must match each object - tracked by a quota. If not specified, the quota matches all - objects. + matchExpressions: + description: A list of scope selector requirements by scope of + the resources. items: - description: A ResourceQuotaScope defines a filter that must - match each object tracked by a quota - type: string + description: A scoped-resource selector requirement is a selector + that contains values, a scope name, and an operator that relates + the scope name and values. + properties: + operator: + description: Represents a scope's relationship to a set + of values. Valid operators are In, NotIn, Exists, DoesNotExist. + type: string + scopeName: + description: The name of the scope that the selector applies + to. + type: string + values: + description: An array of string values. If the operator + is In or NotIn, the values array must be non-empty. If + the operator is Exists or DoesNotExist, the values array + must be empty. This array is replaced during a strategic + merge patch. + items: + type: string + type: array + required: + - operator + - scopeName + type: object type: array type: object + scopes: + description: A collection of filters that must match each object tracked + by a quota. If not specified, the quota matches all objects. + items: + description: A ResourceQuotaScope defines a filter that must match + each object tracked by a quota + type: string + type: array + required: + - matchLabels type: object status: description: Status defines the actual enforced quota and its current @@ -175,8 +174,6 @@ spec: in the namespace. type: object type: object - required: - - namespaces type: object type: object served: true @@ -188,65 +185,72 @@ status: conditions: [] storedVersions: [] --- -apiVersion: admissionregistration.k8s.io/v1beta1 +apiVersion: admissionregistration.k8s.io/v1 kind: MutatingWebhookConfiguration metadata: annotations: - cert-manager.io/inject-ca-from: platform-system/platform-serving-cert + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) creationTimestamp: null name: platform-mutating-webhook-configuration namespace: platform-system webhooks: -- clientConfig: - caBundle: Cg== +- admissionReviewVersions: + - v1 + clientConfig: service: - name: platform-operator + name: webhook-service namespace: platform-system - path: /mutate-v1-ingress + path: /mutate-v1-pod failurePolicy: Ignore - name: annotate-ingress-v1.platform.flanksource.com + name: mutate-v1-pod.platform.flanksource.com rules: - apiGroups: - - extensions + - "" apiVersions: - - v1beta1 + - v1 operations: - CREATE - UPDATE resources: - - ingresses -- clientConfig: - caBundle: Cg== + - pods + sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: service: - name: platform-operator + name: webhook-service namespace: platform-system - path: /mutate-v1-pod + path: /mutate-v1-ingress failurePolicy: Ignore - name: annotate-pods-v1.platform.flanksource.com + name: mutate-v1-pod.platform.flanksource.com rules: - apiGroups: - - "" + - extensions + - networking.k8s.io apiVersions: + - v1beta1 - v1 operations: - CREATE - UPDATE resources: - - pods + - ingresses + sideEffects: None --- -apiVersion: admissionregistration.k8s.io/v1beta1 +apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: annotations: - cert-manager.io/inject-ca-from: platform-system/platform-serving-cert + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) creationTimestamp: null name: platform-validating-webhook-configuration namespace: platform-system webhooks: -- clientConfig: - caBundle: Cg== +- admissionReviewVersions: + - v1 + clientConfig: service: - name: platform-operator + name: webhook-service namespace: platform-system path: /validate-clusterresourcequota-platform-flanksource-com-v1 failurePolicy: Fail @@ -261,12 +265,14 @@ webhooks: - UPDATE resources: - clusterresourcequotas -- clientConfig: - caBundle: Cg== + sideEffects: None +- admissionReviewVersions: + - v1 + clientConfig: service: - name: platform-operator + name: webhook-service namespace: platform-system - path: /validate-resourcequota-v1 + path: /validate-resourcequota-platform-flanksource-com-v1 failurePolicy: Fail name: resourcequotas-validation-v1.platform.flanksource.com rules: @@ -279,6 +285,7 @@ webhooks: - UPDATE resources: - resourcequotas + sideEffects: None --- apiVersion: v1 kind: ServiceAccount @@ -388,10 +395,8 @@ rules: resources: - namespaces verbs: - - delete - get - list - - watch - apiGroups: - "" resources: @@ -428,6 +433,7 @@ rules: - update - apiGroups: - extensions + - networking.k8s.io resources: - ingresses verbs: @@ -544,7 +550,7 @@ spec: defaultMode: 420 secretName: platform-operator --- -apiVersion: cert-manager.io/v1alpha2 +apiVersion: cert-manager.io/v1 kind: Certificate metadata: name: platform-serving-cert @@ -558,7 +564,7 @@ spec: name: platform-selfsigned-issuer secretName: platform-operator --- -apiVersion: cert-manager.io/v1alpha2 +apiVersion: cert-manager.io/v1 kind: Issuer metadata: name: platform-selfsigned-issuer diff --git a/pkg/apis/platform/v1/clusterresourcequota_types.go b/pkg/apis/platform/v1/clusterresourcequota_types.go index 43a27d2..3d6ebca 100644 --- a/pkg/apis/platform/v1/clusterresourcequota_types.go +++ b/pkg/apis/platform/v1/clusterresourcequota_types.go @@ -25,21 +25,18 @@ import ( // ClusterResourceQuotaSpec defines the desired state of ClusterResourceQuota type ClusterResourceQuotaSpec struct { - // Important: Run "make" to regenerate code after modifying this file - - // Quota sets aggregate quota restrictions enforced across all namespaces - Quota corev1.ResourceQuotaSpec `json:"quota,omitempty"` + MatchLabels map[string]string `json:"matchLabels"` + corev1.ResourceQuotaSpec `json:",inline"` } // ClusterResourceQuotaStatus defines the observed state of ClusterResourceQuota type ClusterResourceQuotaStatus struct { - // Important: Run "make" to regenerate code after modifying this file // Total defines the actual enforced quota and its current usage across all namespaces Total corev1.ResourceQuotaStatus `json:"total,omitempty"` // Slices the quota used per namespace - Namespaces ResourceQuotasStatusByNamespace `json:"namespaces"` + Namespaces ResourceQuotasStatusByNamespace `json:"namespaces,omitempty"` } // +kubebuilder:object:root=true diff --git a/pkg/apis/platform/v1/zz_generated.deepcopy.go b/pkg/apis/platform/v1/zz_generated.deepcopy.go index c6c6b00..ebb65ea 100644 --- a/pkg/apis/platform/v1/zz_generated.deepcopy.go +++ b/pkg/apis/platform/v1/zz_generated.deepcopy.go @@ -86,7 +86,14 @@ func (in *ClusterResourceQuotaList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ClusterResourceQuotaSpec) DeepCopyInto(out *ClusterResourceQuotaSpec) { *out = *in - in.Quota.DeepCopyInto(&out.Quota) + if in.MatchLabels != nil { + in, out := &in.MatchLabels, &out.MatchLabels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + in.ResourceQuotaSpec.DeepCopyInto(&out.ResourceQuotaSpec) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterResourceQuotaSpec. diff --git a/pkg/controllers/cleanup/cleanup_controller.go b/pkg/controllers/cleanup/cleanup_controller.go index af91bc8..af04631 100644 --- a/pkg/controllers/cleanup/cleanup_controller.go +++ b/pkg/controllers/cleanup/cleanup_controller.go @@ -95,7 +95,6 @@ func parseDuration(expiry string) (*time.Duration, error) { } // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch;delete - func (r *ReconcileCleanup) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { namespace := corev1.Namespace{} if err := r.Get(ctx, request.NamespacedName, &namespace); err != nil { diff --git a/pkg/controllers/clusterresourcequota/clusterresourcequota_controller.go b/pkg/controllers/clusterresourcequota/clusterresourcequota_controller.go index 836da8b..6ef8dc4 100644 --- a/pkg/controllers/clusterresourcequota/clusterresourcequota_controller.go +++ b/pkg/controllers/clusterresourcequota/clusterresourcequota_controller.go @@ -18,6 +18,10 @@ package clusterresourcequota import ( "context" + "math/rand" + "strings" + "sync" + "time" platformv1 "github.com/flanksource/platform-operator/pkg/apis/platform/v1" @@ -25,7 +29,6 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - utilquota "k8s.io/apiserver/pkg/quota/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -47,6 +50,7 @@ func Add(mgr manager.Manager) error { func newReconciler(mgr manager.Manager) reconcile.Reconciler { return &ReconcileClusterResourceQuota{ + mtx: &sync.Mutex{}, Client: mgr.GetClient(), Scheme: mgr.GetScheme(), } @@ -95,6 +99,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { var _ reconcile.Reconciler = &ReconcileClusterResourceQuota{} type ReconcileClusterResourceQuota struct { + mtx *sync.Mutex client.Client Scheme *runtime.Scheme } @@ -104,81 +109,42 @@ type ReconcileClusterResourceQuota struct { // +kubebuilder:rbac:groups="",resources=resourcequotas,verbs=get;list;watch func (r *ReconcileClusterResourceQuota) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { + r.mtx.Lock() + defer r.mtx.Unlock() + quota := &platformv1.ClusterResourceQuota{} if err := r.Get(ctx, request.NamespacedName, quota); err != nil { if apierrors.IsNotFound(err) { return reconcile.Result{}, nil } - - log.Error(err, "Failed to get Cluster Resource Quota") return reconcile.Result{}, err } - namespacesList := &corev1.NamespaceList{} - if err := r.List(ctx, namespacesList); err != nil { - log.Error(err, "Failed to list namespaces") + existing, err := findMatchingResourceQuotas(ctx, r.Client, quota, nil) + if err != nil { return reconcile.Result{}, err } - for _, namespace := range namespacesList.Items { - namespaceName := namespace.Name - - // get the quotas per namespace in the status of the ClusterQuotaResource - // this represent the past - here below we need to compute the future - namespaceTotals := GetResourceQuotasStatusByNamespace(quota.Status.Namespaces, namespaceName) - - rqList := &corev1.ResourceQuotaList{} - if err := r.List(ctx, rqList, client.InNamespace(namespaceName)); err != nil { - log.Error(err, "Failed to list Resource Quota", "namespace", namespaceName) - return reconcile.Result{}, err - } - - if len(rqList.Items) == 0 { - log.Info("Warning: no ResourceQuota defined", "namespace", namespaceName) - continue - } - - // calculate the quotas on a single namespace - var recalculatedStatus corev1.ResourceQuotaStatus = corev1.ResourceQuotaStatus{} - for _, rq := range rqList.Items { - // calculate the status across all the Resource Quota in a namespace - usedCurrent := utilquota.Add(recalculatedStatus.Used, rq.Status.Used) - hardCurrent := utilquota.Add(recalculatedStatus.Hard, rq.Status.Hard) - - recalculatedStatus = corev1.ResourceQuotaStatus{ - Used: usedCurrent, - Hard: hardCurrent, - } - } + quota.Status.Total.Hard = sumOfHard(existing) + quota.Status.Total.Used = sumOfUsed(existing) + quota.Status.Namespaces = platformv1.ResourceQuotasStatusByNamespace{} + sum, keys := sumByNamespace(existing) - // subtract old usage, add new usage - quota.Status.Total.Used = utilquota.Subtract(quota.Status.Total.Used, namespaceTotals.Used) - quota.Status.Total.Used = utilquota.Add(quota.Status.Total.Used, recalculatedStatus.Used) - InsertResourceQuotasStatus("a.Status.Namespaces, platformv1.ResourceQuotaStatusByNamespace{ - Namespace: namespaceName, - Status: recalculatedStatus, + for _, namespace := range keys { + quota.Status.Namespaces = append(quota.Status.Namespaces, platformv1.ResourceQuotaStatusByNamespace{ + Namespace: namespace, + Status: sum[namespace].Status, }) } - statusCopy := quota.Status.Namespaces.DeepCopy() - for _, namespaceTotals := range statusCopy { - namespaceName := namespaceTotals.Namespace - - rqList := &corev1.ResourceQuotaList{} - if err := r.List(ctx, rqList, client.InNamespace(namespaceName)); err != nil { - log.Error(err, "Failed to list Resource Quota", "namespace", namespaceName) - return reconcile.Result{}, err + if err := r.Client.Status().Update(ctx, quota); err != nil { + if strings.Contains(err.Error(), "the object has been modified; please apply your changes to the latest version and try again") { + log.Info("Concurrent update detected, retrying") + return reconcile.Result{RequeueAfter: time.Second * time.Duration(1+rand.Intn(4))}, nil } - - if len(rqList.Items) == 0 { - quota.Status.Total.Used = utilquota.Subtract(quota.Status.Total.Used, namespaceTotals.Status.Used) - RemoveResourceQuotasStatusByNamespace("a.Status.Namespaces, namespaceName) + if apierrors.IsNotFound(err) { + return reconcile.Result{}, nil } - } - - quota.Status.Total.Hard = quota.Spec.Quota.Hard - if err := r.Client.Update(ctx, quota); err != nil { - log.Error(err, "Failed to update status") return reconcile.Result{}, err } diff --git a/pkg/controllers/clusterresourcequota/clusterresourcequota_controller_test.go b/pkg/controllers/clusterresourcequota/clusterresourcequota_controller_test.go index 1339793..59a50ae 100644 --- a/pkg/controllers/clusterresourcequota/clusterresourcequota_controller_test.go +++ b/pkg/controllers/clusterresourcequota/clusterresourcequota_controller_test.go @@ -61,7 +61,7 @@ func TestReconcileClusterResourceQuota_doReconcile(t *testing.T) { Name: "clusterresourcequota-sample", }, Spec: platformv1.ClusterResourceQuotaSpec{ - Quota: corev1.ResourceQuotaSpec{ + ResourceQuotaSpec: corev1.ResourceQuotaSpec{ Hard: corev1.ResourceList{ corev1.ResourcePods: resource.MustParse("10"), }, diff --git a/pkg/controllers/clusterresourcequota/clusterresourcequota_validatingwebhook.go b/pkg/controllers/clusterresourcequota/clusterresourcequota_validatingwebhook.go index 582c51a..cc34647 100644 --- a/pkg/controllers/clusterresourcequota/clusterresourcequota_validatingwebhook.go +++ b/pkg/controllers/clusterresourcequota/clusterresourcequota_validatingwebhook.go @@ -20,17 +20,14 @@ import ( "context" "fmt" "net/http" + "strings" "sync" platformv1 "github.com/flanksource/platform-operator/pkg/apis/platform/v1" - corev1 "k8s.io/api/core/v1" - utilquota "k8s.io/apiserver/pkg/quota/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// +kubebuilder:webhook:path=/validate-clusterresourcequota-platform-flanksource-com-v1,mutating=false,failurePolicy=fail,groups=platform.flanksource.com,resources=clusterresourcequotas,verbs=create;update,versions=v1,name=clusterresourcequotas-validation-v1.platform.flanksource.com - func NewClusterResourceQuotaValidatingWebhook(client client.Client, mtx *sync.Mutex, validationEnabled bool) *admission.Webhook { decoder, _ := admission.NewDecoder(client.Scheme()) return &admission.Webhook{ @@ -56,9 +53,9 @@ func (v *validatingClusterResourceQuotaHandler) Handle(ctx context.Context, req v.mtx.Lock() defer v.mtx.Unlock() - quota := &platformv1.ClusterResourceQuota{} + crq := &platformv1.ClusterResourceQuota{} - if err := v.Decode(req, quota); err != nil { + if err := v.Decode(req, crq); err != nil { return admission.Errored(http.StatusBadRequest, err) } @@ -67,35 +64,19 @@ func (v *validatingClusterResourceQuotaHandler) Handle(ctx context.Context, req return admission.Allowed("") } - namespacesList := &corev1.NamespaceList{} - if err := v.List(ctx, namespacesList); err != nil { - log.Error(err, "Failed to list namespaces") + existing, err := findMatchingResourceQuotas(ctx, v.Client, crq, nil) + if err != nil { return admission.Errored(http.StatusBadRequest, err) } - // store here the total hard of all resource quotas - hardTotals := corev1.ResourceList{} - for _, namespace := range namespacesList.Items { - namespaceName := namespace.Name - - rqList := &corev1.ResourceQuotaList{} - if err := v.List(ctx, rqList, client.InNamespace(namespaceName)); err != nil { - log.Error(err, "Failed to list Resource Quota", "namespace", namespaceName) - return admission.Errored(http.StatusBadRequest, err) - } + used := sumOfHard(existing) - if len(rqList.Items) == 0 { - continue + if isOk, rn := greaterThan(used, crq.Spec.Hard); !isOk { + msg := "" + for _, resource := range rn { + msg += fmt.Sprintf(" %s(%s > %s)", resource, qtyString(used[resource]), qtyString(crq.Spec.Hard[resource])) } - - for _, rq := range rqList.Items { - hardTotals = utilquota.Add(hardTotals, rq.Status.Hard) - } - } - - // check quota - if isOk, rn := utilquota.LessThanOrEqual(hardTotals, quota.Spec.Quota.Hard); !isOk { - return admission.Denied(fmt.Sprintf("total resource quotas exceeed cluster resource quota hard limits %v", rn)) + return admission.Denied(fmt.Sprintf("cannot update ClusterResourceQuota/%s it would be below current usage: %s", crq.Name, strings.TrimSpace(msg))) } return admission.Allowed("") diff --git a/pkg/controllers/clusterresourcequota/resourcequota_validatingwebhook.go b/pkg/controllers/clusterresourcequota/resourcequota_validatingwebhook.go index a8c5b78..7ba3ea0 100644 --- a/pkg/controllers/clusterresourcequota/resourcequota_validatingwebhook.go +++ b/pkg/controllers/clusterresourcequota/resourcequota_validatingwebhook.go @@ -20,17 +20,18 @@ import ( "context" "fmt" "net/http" + "strings" "sync" - platformv1 "github.com/flanksource/platform-operator/pkg/apis/platform/v1" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" utilquota "k8s.io/apiserver/pkg/quota/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) -// +kubebuilder:webhook:path=/validate-resourcequota-v1,mutating=false,failurePolicy=fail,groups="",resources=resourcequotas,verbs=create;update,versions=v1,name=resourcequotas-validation-v1.platform.flanksource.com +// +kubebuilder:webhook:path=/validate-resourcequota-v1,mutating=false,sideEffects=None,admissionReviewVersions=v1,failurePolicy=fail,groups="",resources=resourcequotas,verbs=create;update,versions=v1,name=resourcequotas-validation-v1.platform.flanksource.com func NewResourceQuotaValidatingWebhook(client client.Client, mtx *sync.Mutex, validationEnabled bool) *admission.Webhook { decoder, _ := admission.NewDecoder(client.Scheme()) return &admission.Webhook{ @@ -61,56 +62,37 @@ func (v *validatingResourceQuotaHandler) Handle(ctx context.Context, req admissi return admission.Errored(http.StatusBadRequest, err) } - if !v.validationEnabled { - log.Info("validate resource quota flag is not enabled. All requests will be declared valid") - return admission.Allowed("") + var namespace v1.Namespace + if err := v.Client.Get(ctx, namespaceKey(rq), &namespace); err != nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("cannot find namespace for resource quota: %s", rq.Namespace)) } - namespacesList := &corev1.NamespaceList{} - if err := v.List(ctx, namespacesList); err != nil { - log.Error(err, "Failed to list namespaces") + crq, err := findClusterResourceQuota(ctx, v.Client, namespace) + if err != nil { return admission.Errored(http.StatusBadRequest, err) } - - // store here the total hard of all resource quotas - hardTotals := corev1.ResourceList{} - for _, namespace := range namespacesList.Items { - namespaceName := namespace.Name - - rqList := &corev1.ResourceQuotaList{} - if err := v.List(ctx, rqList, client.InNamespace(namespaceName)); err != nil { - log.Error(err, "Failed to list Resource Quota", "namespace", namespaceName) - return admission.Errored(http.StatusBadRequest, err) - } - - if len(rqList.Items) == 0 { - continue - } - - for _, rq := range rqList.Items { - hardTotals = utilquota.Add(hardTotals, rq.Status.Hard) - } + if crq == nil { + return admission.Allowed("") } - hardTotals = utilquota.Add(hardTotals, rq.Spec.Hard) + if !v.validationEnabled { + log.Info("validate resource quota flag is not enabled. All requests will be declared valid") + return admission.Allowed("") + } - // in case in the cluster we define multiple resource quotas - // NOTE: in the future we could have cluster resource quotas applied to - // some namespaces - quotaList := &platformv1.ClusterResourceQuotaList{} - if err := v.List(ctx, quotaList); err != nil { - log.Error(err, "Failed to list cluster resource quotas") + existing, err := findMatchingResourceQuotas(ctx, v.Client, crq, rq) + if err != nil { return admission.Errored(http.StatusBadRequest, err) } - if len(quotaList.Items) == 0 { - admission.Allowed("") - } + sum := sumOfHard(append(existing, *rq)) - for _, q := range quotaList.Items { - if isOk, rn := utilquota.LessThanOrEqual(hardTotals, q.Spec.Quota.Hard); !isOk { - return admission.Denied(fmt.Sprintf("resource quota exceeds the cluster resource quota %s in %v", q.Name, rn)) + if isOk, rn := utilquota.LessThanOrEqual(sum, crq.Spec.Hard); !isOk { + msg := "" + for _, resource := range rn { + msg += fmt.Sprintf(" %s(%s > %s)", resource, qtyString(sum[resource]), qtyString(crq.Spec.Hard[resource])) } + return admission.Denied(fmt.Sprintf("ResourceQuota/%s/%s would exceed ClusterResourceQuota/%s: %s", rq.Namespace, rq.Name, crq.Name, strings.TrimSpace(msg))) } return admission.Allowed("") } diff --git a/pkg/controllers/clusterresourcequota/resources.go b/pkg/controllers/clusterresourcequota/resources.go index 641a14a..d3d991d 100644 --- a/pkg/controllers/clusterresourcequota/resources.go +++ b/pkg/controllers/clusterresourcequota/resources.go @@ -17,11 +17,125 @@ limitations under the License. package clusterresourcequota import ( + "context" + "sort" + platformv1 "github.com/flanksource/platform-operator/pkg/apis/platform/v1" + "sigs.k8s.io/controller-runtime/pkg/client" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + utilquota "k8s.io/apiserver/pkg/quota/v1" ) +func findClusterResourceQuota(ctx context.Context, client client.Client, namespace v1.Namespace) (*platformv1.ClusterResourceQuota, error) { + quotaList := &platformv1.ClusterResourceQuotaList{} + err := client.List(ctx, quotaList) + if err != nil { + return nil, err + } + for _, quota := range quotaList.Items { + if matches(namespace, "a) { + return "a, nil + } + } + + return nil, nil +} + +// findMatchingResourceQuotas returns all resource quotas matched by a cluster resource quota +// excluding the resource quota being worked on +func findMatchingResourceQuotas(ctx context.Context, c client.Client, crq *platformv1.ClusterResourceQuota, existing *corev1.ResourceQuota) ([]corev1.ResourceQuota, error) { + var namespaces v1.NamespaceList + if err := c.List(ctx, &namespaces, client.MatchingLabels(crq.Spec.MatchLabels)); err != nil { + return nil, err + } + + resources := []corev1.ResourceQuota{} + + for _, namespace := range namespaces.Items { + namespaceName := namespace.Name + rqList := &corev1.ResourceQuotaList{} + if err := c.List(ctx, rqList, client.InNamespace(namespaceName)); err != nil { + return nil, err + } + + if len(rqList.Items) == 0 { + continue + } + + for _, item := range rqList.Items { + if existing != nil && (item.GetNamespace() == existing.GetNamespace() && item.GetName() == existing.GetName()) { + continue + } + resources = append(resources, item) + } + + } + return resources, nil +} + +func sumByNamespace(list []corev1.ResourceQuota) (map[string]corev1.ResourceQuota, []string) { + sum := map[string]corev1.ResourceQuota{} + keys := []string{} + for _, item := range list { + + if _, ok := sum[item.GetNamespace()]; !ok { + keys = append(keys, item.GetNamespace()) + sum[item.GetNamespace()] = corev1.ResourceQuota{ + ObjectMeta: metav1.ObjectMeta{Namespace: item.GetNamespace()}, + Spec: v1.ResourceQuotaSpec{}, + Status: v1.ResourceQuotaStatus{}, + } + } + resource := sum[item.GetNamespace()] + resource.Spec.Hard = utilquota.Add(resource.Spec.Hard, item.Spec.Hard) + resource.Status.Used = utilquota.Add(resource.Status.Used, item.Status.Used) + } + sort.Strings(keys) + return sum, keys +} + +func sumOfHard(list []corev1.ResourceQuota) corev1.ResourceList { + sum := corev1.ResourceList{} + for _, item := range list { + sum = utilquota.Add(sum, item.Spec.Hard) + } + return sum +} + +func sumOfUsed(list []corev1.ResourceQuota) corev1.ResourceList { + sum := corev1.ResourceList{} + for _, item := range list { + sum = utilquota.Add(sum, item.Status.Used) + } + return sum +} + +func namespaceKey(obj metav1.Object) types.NamespacedName { + return types.NamespacedName{ + Name: obj.GetNamespace(), + } +} + +// greaterThan returns true if a > b for any key in b +// If false, it returns the keys in a that exceeded b +func greaterThan(a corev1.ResourceList, b corev1.ResourceList) (bool, []corev1.ResourceName) { + result := true + resourceNames := []corev1.ResourceName{} + for key, value := range b { + if other, found := a[key]; found { + if other.Cmp(value) == 1 { + result = false + resourceNames = append(resourceNames, key) + } + } + } + return result, resourceNames +} + func GetResourceQuotasStatusByNamespace(namespaceStatuses platformv1.ResourceQuotasStatusByNamespace, namespace string) corev1.ResourceQuotaStatus { for i := range namespaceStatuses { curr := namespaceStatuses[i] diff --git a/pkg/controllers/clusterresourcequota/utils.go b/pkg/controllers/clusterresourcequota/utils.go new file mode 100644 index 0000000..6dbab7e --- /dev/null +++ b/pkg/controllers/clusterresourcequota/utils.go @@ -0,0 +1,40 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package clusterresourcequota + +import ( + platformv1 "github.com/flanksource/platform-operator/pkg/apis/platform/v1" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func matches(namespace corev1.Namespace, quota *platformv1.ClusterResourceQuota) bool { + if len(quota.Spec.MatchLabels) == 0 { + return false + } + + for k, v := range quota.Spec.MatchLabels { + if namespace.GetLabels()[k] != v { + return false + } + } + return true +} + +func qtyString(v resource.Quantity) string { + return v.String() +} diff --git a/pkg/controllers/ingress/ingress_reconciler.go b/pkg/controllers/ingress/ingress_reconciler.go index 1b6e3a0..4e8cbba 100644 --- a/pkg/controllers/ingress/ingress_reconciler.go +++ b/pkg/controllers/ingress/ingress_reconciler.go @@ -91,7 +91,7 @@ func addIngressReconciler(mgr manager.Manager, r reconcile.Reconciler) error { return nil } -// +kubebuilder:rbac:groups="extensions",resources=ingresses,verbs=get;list;update;watch +// +kubebuilder:rbac:groups=extensions;networking.k8s.io,resources=ingresses,verbs=get;list;update;watch func (r *IngressReconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { ingress := &v1beta1.Ingress{} diff --git a/pkg/controllers/ingress/mutating_webhook.go b/pkg/controllers/ingress/mutating_webhook.go index c815b6c..ed720b1 100644 --- a/pkg/controllers/ingress/mutating_webhook.go +++ b/pkg/controllers/ingress/mutating_webhook.go @@ -30,7 +30,7 @@ func NewMutatingWebhook(client client.Client, svcName, svcNamespace, domain stri } } -// +kubebuilder:webhook:path=/mutate-v1-ingress,mutating=true,failurePolicy=ignore,groups="extensions",resources=ingresses,verbs=create;update,versions=v1beta1,name=annotate-ingress-v1.platform.flanksource.com +// +kubebuilder:webhook:path=/mutate-v1-ingress,mutating=true,sideEffects=None,admissionReviewVersions=v1,failurePolicy=ignore,groups="extensions",resources=ingresses,verbs=create;update,versions=v1beta1,name=mutate-ingress-v1.platform.flanksource.com func (handler *ingressHandler) Handle(ctx context.Context, req admission.Request) admission.Response { ingress := &v1beta1.Ingress{} diff --git a/pkg/controllers/pod/mutating_webhook.go b/pkg/controllers/pod/mutating_webhook.go index 91b4d2b..85c9b0b 100644 --- a/pkg/controllers/pod/mutating_webhook.go +++ b/pkg/controllers/pod/mutating_webhook.go @@ -25,6 +25,7 @@ type podHandler struct { platformv1.PodMutaterConfig } +//+kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,sideEffects=None,admissionReviewVersions=v1,failurePolicy=ignore,groups="",resources=pods,verbs=create;update,versions=v1,name=mutate-pods-v1.platform.flanksource.com func NewMutatingWebhook(client client.Client, cfg platformv1.PodMutaterConfig) *admission.Webhook { cfg.AnnotationsMap = make(map[string]bool) for _, a := range cfg.Annotations { @@ -40,7 +41,6 @@ func NewMutatingWebhook(client client.Client, cfg platformv1.PodMutaterConfig) * } } -// +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=ignore,groups="",resources=pods,verbs=create;update,versions=v1,name=annotate-pods-v1.platform.flanksource.com func (handler *podHandler) Handle(ctx context.Context, req admission.Request) admission.Response { pod := &corev1.Pod{} err := handler.Decode(req, pod) diff --git a/test/clusterresourcequota_test.go b/test/clusterresourcequota_test.go index 6859b93..1e9ddde 100644 --- a/test/clusterresourcequota_test.go +++ b/test/clusterresourcequota_test.go @@ -15,99 +15,125 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +var matchBy = map[string]string{"name": "n1"} + +func newClusterResourceQuota() *platformv1.ClusterResourceQuota { + return &platformv1.ClusterResourceQuota{ + TypeMeta: metav1.TypeMeta{APIVersion: "platform.flanksource.com/v1", Kind: "ClusterResourceQuota"}, + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("crq-%s", utils.RandomString(3))}, + Status: platformv1.ClusterResourceQuotaStatus{ + Namespaces: platformv1.ResourceQuotasStatusByNamespace{}, + }, + Spec: platformv1.ClusterResourceQuotaSpec{ + MatchLabels: matchBy, + ResourceQuotaSpec: v1.ResourceQuotaSpec{ + Hard: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("2"), + v1.ResourceMemory: resource.MustParse("2Gi"), + }, + }, + }, + } +} + var _ = Describe("ClusterResourceQuota Controller", func() { + var ctx = context.Background() const timeout = time.Second * 30 const interval = time.Second * 1 + n1 := v1.Namespace{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Namespace"}, + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("ns-with-clusterquota-%s", utils.RandomString(3)), Labels: matchBy}, + } - var clusterResourceQuota *platformv1.ClusterResourceQuota - var resourceQuotas = []v1.ResourceQuota{} - var namespaces = []v1.Namespace{} + n2 := v1.Namespace{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Namespace"}, + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("ns-with-clusterquota-%s", utils.RandomString(3)), Labels: matchBy}, + } - AfterEach(func() { - if clusterResourceQuota != nil { - _ = k8sClient.Delete(context.Background(), clusterResourceQuota) - } - for _, r := range resourceQuotas { - _ = k8sClient.Delete(context.Background(), &r) - } - for _, n := range namespaces { - _ = k8sClient.Delete(context.Background(), &n) + var crq *platformv1.ClusterResourceQuota + + CreateQuota := func(namespace, cpu, memory string) (v1.ResourceQuota, error) { + r := v1.ResourceQuota{ + TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "ResourceQuota"}, + ObjectMeta: metav1.ObjectMeta{Name: "rq", Namespace: namespace}, + Spec: v1.ResourceQuotaSpec{ + Hard: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse(cpu), + v1.ResourceMemory: resource.MustParse(memory), + }, + }, } - }) + err := k8sClient.Create(ctx, &r) + return r, err + } - XContext("ClusterResourceQuota exists", func() { - It("allows ResourceQuota creation within limits", func() { - n1 := v1.Namespace{ - TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Namespace"}, - ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("ns-with-annotations-%s", utils.RandomString(3))}, - } - n2 := v1.Namespace{ - TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Namespace"}, - ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("ns-with-annotations-%s", utils.RandomString(3))}, - } - namespaces = append(namespaces, n1, n2) - err := k8sClient.Create(context.Background(), &n1) + Describe("ClusterResourceQuota", func() { + + It("setup", func() { + err := k8sClient.Create(ctx, &n1) Expect(err).ToNot(HaveOccurred()) - err = k8sClient.Create(context.Background(), &n2) + err = k8sClient.Create(ctx, &n2) Expect(err).ToNot(HaveOccurred()) + }) - crq := platformv1.ClusterResourceQuota{ - TypeMeta: metav1.TypeMeta{APIVersion: "platform.flanksource.com/v1", Kind: "ClusterResourceQuota"}, - ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("crq-%s", utils.RandomString(3))}, - Status: platformv1.ClusterResourceQuotaStatus{ - Namespaces: platformv1.ResourceQuotasStatusByNamespace{}, - }, - Spec: platformv1.ClusterResourceQuotaSpec{ - Quota: v1.ResourceQuotaSpec{ - Hard: v1.ResourceList{ - v1.ResourceCPU: resource.MustParse("2"), - v1.ResourceMemory: resource.MustParse("1Gi"), - }, - }, - }, - } - err = k8sClient.Create(context.Background(), &crq) + BeforeEach(func() { + crq = newClusterResourceQuota() + err := k8sClient.Create(ctx, crq) Expect(err).ToNot(HaveOccurred()) + }) - r1 := v1.ResourceQuota{ - TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "ResourceQuota"}, - ObjectMeta: metav1.ObjectMeta{Name: "rq", Namespace: n1.Name}, - Spec: v1.ResourceQuotaSpec{ - Hard: v1.ResourceList{ - v1.ResourceCPU: resource.MustParse("3"), - v1.ResourceMemory: resource.MustParse("1500Mi"), - }, - }, + AfterEach(func() { + err := k8sClient.Delete(ctx, crq) + Expect(err).ToNot(HaveOccurred()) + rqList := v1.ResourceQuotaList{} + err = k8sClient.List(ctx, &rqList) + Expect(err).ToNot(HaveOccurred()) + + for _, rq := range rqList.Items { + err = k8sClient.Delete(ctx, &rq) + Expect(err).ToNot(HaveOccurred()) } - err = k8sClient.Create(context.Background(), &r1) + }) + + It("allows ResourceQuota creation within limits", func() { + _, err := CreateQuota(n1.Name, "900m", "500Mi") Expect(err).ToNot(HaveOccurred()) + _, err = CreateQuota(n2.Name, "1000m", "1Gi") + Expect(err).ToNot(HaveOccurred()) + }) - r2 := v1.ResourceQuota{ - TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "ResourceQuota"}, - ObjectMeta: metav1.ObjectMeta{Name: "rq", Namespace: n2.Name}, - Spec: v1.ResourceQuotaSpec{ - Hard: v1.ResourceList{ - v1.ResourceCPU: resource.MustParse("1"), - v1.ResourceMemory: resource.MustParse("600Mi"), - }, - }, + XIt("should update its status", func() { + }) + + It("should not allow updating to lower than ResourceQuota", func() { + _, err := CreateQuota(n1.Name, "1100m", "1Gi") + Expect(err).ToNot(HaveOccurred()) + crq.Spec.Hard = v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("500m"), + v1.ResourceMemory: resource.MustParse("2Gi"), } - err = k8sClient.Create(context.Background(), &r2) + err = k8sClient.Update(ctx, crq) Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("cpu(1100m > 500m)")) }) - }) - Context("ClusterResourceQuota exists", func() { - It("allows ResourceQuota creation within limits", func() { - rq := v1.ResourceQuotaList{} - err := k8sClient.List(context.Background(), &rq) + It("should allow updating to higher than ResourceQuota", func() { + _, err := CreateQuota(n1.Name, "1000m", "1Gi") Expect(err).ToNot(HaveOccurred()) + crq.Spec.Hard = v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("1500m"), + v1.ResourceMemory: resource.MustParse("2Gi"), + } + err = k8sClient.Update(ctx, crq) + Expect(err).ToNot(HaveOccurred()) + }) - // create two resource quotas - // check if cluster resource quota can be created only withing limits - // TODO: currently we can't delete resource quotas created by previous tests - // Using testEnv resources cannot be properly deleted unfortunately + It("should not allow ResourceQuota creation outside of limits", func() { + _, err := CreateQuota(n1.Name, "1000m", "1Gi") + Expect(err).ToNot(HaveOccurred()) + _, err = CreateQuota(n2.Name, "1500m", "1Gi") + Expect(err).To(HaveOccurred()) }) }) }) diff --git a/test/suite_test.go b/test/suite_test.go index 449a412..4c726fe 100644 --- a/test/suite_test.go +++ b/test/suite_test.go @@ -8,6 +8,7 @@ import ( "net" "os" "path/filepath" + "sync" "testing" "time" @@ -79,7 +80,7 @@ func waitFor(host string) { }, 60*time.Second, 1*time.Second).Should(Succeed()) } -func registerWebhook(manager ctrl.Manager, name string, webhook *admission.Webhook) error { +func registerWebhook(manager ctrl.Manager, name string, webhook *admission.Webhook, apiGroup, apiVersion string, resources ...string) error { wh := &admissionregistrationv1beta1.MutatingWebhookConfiguration{} wh.Name = name _, err := ctrl.CreateOrUpdate(context.TODO(), manager.GetClient(), wh, func() error { @@ -100,9 +101,9 @@ func registerWebhook(manager ctrl.Manager, name string, webhook *admission.Webho admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update, }, Rule: admissionregistrationv1beta1.Rule{ - APIGroups: []string{""}, - APIVersions: []string{"v1"}, - Resources: []string{"pods"}, + APIGroups: []string{apiGroup}, + APIVersions: []string{apiVersion}, + Resources: resources, }, }, }, @@ -179,7 +180,19 @@ var _ = BeforeSuite(func(done Done) { }() By("Waiting for webhook server to come up") waitFor(fmt.Sprintf("localhost:%d", port)) - err = registerWebhook(k8sManager, "annotate-pods-v1.platform.flanksource.com", &webhook.Admission{Handler: pod.NewMutatingWebhook(k8sManager.GetClient(), podConfig)}) + err = registerWebhook(k8sManager, "annotate-pods-v1.platform.flanksource.com", + &webhook.Admission{Handler: pod.NewMutatingWebhook(k8sManager.GetClient(), podConfig)}, + "", "v1", "pods") + Expect(err).ToNot(HaveOccurred()) + + err = registerWebhook(k8sManager, "clusterresourcequota-v1.platform.flanksource.com", + &webhook.Admission{Handler: clusterresourcequota.NewClusterResourceQuotaValidatingWebhook(k8sManager.GetClient(), &sync.Mutex{}, true)}, + "platform.flanksource.com", "v1", "clusterresourcequotas") + Expect(err).ToNot(HaveOccurred()) + + err = registerWebhook(k8sManager, "resourcequota-v1.platform.flanksource.com", + &webhook.Admission{Handler: clusterresourcequota.NewResourceQuotaValidatingWebhook(k8sManager.GetClient(), &sync.Mutex{}, true)}, + "", "v1", "resourcequotas") Expect(err).ToNot(HaveOccurred()) By("Webhook server is up") k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) From 82615c0548d79dad8eceecbcc69e4656f5b39fd1 Mon Sep 17 00:00:00 2001 From: Moshe Immerman Date: Sun, 2 May 2021 11:43:40 +0000 Subject: [PATCH 3/4] fix: tests --- test/clusterresourcequota_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/clusterresourcequota_test.go b/test/clusterresourcequota_test.go index 1e9ddde..7a672f9 100644 --- a/test/clusterresourcequota_test.go +++ b/test/clusterresourcequota_test.go @@ -103,8 +103,7 @@ var _ = Describe("ClusterResourceQuota Controller", func() { Expect(err).ToNot(HaveOccurred()) }) - XIt("should update its status", func() { - }) + //TODO: test CRQ status It("should not allow updating to lower than ResourceQuota", func() { _, err := CreateQuota(n1.Name, "1100m", "1Gi") From 712a6462669615d6b5466a47455f85295df44d73 Mon Sep 17 00:00:00 2001 From: Moshe Immerman Date: Sun, 2 May 2021 12:00:08 +0000 Subject: [PATCH 4/4] test: remove outdated unit test --- .../clusterresourcequota_controller_test.go | 121 ------------------ 1 file changed, 121 deletions(-) delete mode 100644 pkg/controllers/clusterresourcequota/clusterresourcequota_controller_test.go diff --git a/pkg/controllers/clusterresourcequota/clusterresourcequota_controller_test.go b/pkg/controllers/clusterresourcequota/clusterresourcequota_controller_test.go deleted file mode 100644 index 59a50ae..0000000 --- a/pkg/controllers/clusterresourcequota/clusterresourcequota_controller_test.go +++ /dev/null @@ -1,121 +0,0 @@ -/* -Copyright 2020 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package clusterresourcequota - -import ( - "context" - "reflect" - "testing" - - platformv1 "github.com/flanksource/platform-operator/pkg/apis/platform/v1" - - corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" - ctrl "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/client/fake" -) - -func setupScheme() *runtime.Scheme { - scheme := runtime.NewScheme() - - if err := platformv1.AddToScheme(scheme); err != nil { - panic(err) - } - - if err := corev1.AddToScheme(scheme); err != nil { - panic(err) - } - - return scheme -} - -func TestReconcileClusterResourceQuota_doReconcile(t *testing.T) { - testCases := []struct { - name string - cr *platformv1.ClusterResourceQuota - // external objects like Resource Quota - objects []runtime.Object - quotaStatusWanted platformv1.ClusterResourceQuotaStatus - }{ - { - name: "if no resource quota is present in any namespace, return status per namespace empty", - cr: &platformv1.ClusterResourceQuota{ - ObjectMeta: metav1.ObjectMeta{ - Name: "clusterresourcequota-sample", - }, - Spec: platformv1.ClusterResourceQuotaSpec{ - ResourceQuotaSpec: corev1.ResourceQuotaSpec{ - Hard: corev1.ResourceList{ - corev1.ResourcePods: resource.MustParse("10"), - }, - }, - }, - }, - objects: []runtime.Object{}, - quotaStatusWanted: platformv1.ClusterResourceQuotaStatus{ - Total: corev1.ResourceQuotaStatus{ - Hard: corev1.ResourceList{ - corev1.ResourcePods: resource.MustParse("10"), - }, - }, - }, - }, - } - - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { - // TODO: @mazzy89 Refactor in favor of pkg/eventest - // Deprecated: This package will be dropped before the v1.0.0 release. Package fake provides a fake client for testing. - k8sFakeClient := fake.NewFakeClientWithScheme(setupScheme(), append(tt.objects, tt.cr)...) - r := &ReconcileClusterResourceQuota{ - Client: k8sFakeClient, - Scheme: setupScheme(), - } - - request := ctrl.Request{ - NamespacedName: types.NamespacedName{ - Namespace: tt.cr.GetNamespace(), - Name: tt.cr.GetName(), - }, - } - - result, err := r.Reconcile(context.Background(), request) - if err != nil { - t.Error(err) - } - - if result.Requeue != false { - t.Errorf("Expected no requeue, got %v", result.Requeue) - } - - quota := &platformv1.ClusterResourceQuota{} - if err := r.Client.Get(context.TODO(), types.NamespacedName{ - Namespace: tt.cr.GetNamespace(), - Name: tt.cr.GetName(), - }, quota); err != nil { - t.Error(err) - } - - if !reflect.DeepEqual(quota.Status, tt.quotaStatusWanted) { - t.Errorf("Expected quota status %v got %v", tt.quotaStatusWanted, quota.Status) - } - }) - } -}