Merge pull request #78 from vshn/fix/quota

Fix resource calculation by taking sidecars into account

docs/modules/ROOT/pages/explanations/webhooks.adoc

@@ -5,6 +5,7 @@ APPUiO Cloud provides a quota system, which AppCat has to comply and integrate i
 It accomplishes that, by providing nice error messages, if any of the given quotas are breached.
 
 There are multiple quotas that need to be complied by this system:
+
 * the amount of namespaces an organization can have
 * the amount of resources that can be used within a namespace
 
@@ -16,8 +17,11 @@ If the requested AppCat instance would tip the amount of namespaces over this th
 Additionally, each namespace has quotas that need to comply.
 Via AppCat comp-functions, we set the quotas to a slightly higher value than the APPUiO Cloud default:
 
-* 4.5 CPU limts/requests
-* 16.5Gb Memory limits/requests
+* 4.5 CPU limits/requests + limits/requests of all sidecars
+* 16.5Gb Memory limits/requests + limits/requests of all sidecars
+
+The limits/requests of the sidecars are automatically calculated based on the defined limits/requests of all involved sidecars.
+They are defined in the `sideCars` section of the service in the https://github.com/vshn/component-appcat/blob/master/class/defaults.yml[componet-appcat].
 
 This should ensure that for each service at least one replica can be instantiated with the `*-8` plans.
 These values are written into annotations on the namespace.

pkg/common/quotas/quotas.go

@@ -6,6 +6,7 @@ import (
 	"strconv"
 
 	"github.com/vshn/appcat/v4/pkg/common/utils"
+	iofruntime "github.com/vshn/appcat/v4/pkg/comp-functions/runtime"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
@@ -25,12 +26,13 @@ type QuotaChecker struct {
 	gr                  schema.GroupResource
 	gk                  schema.GroupKind
 	checkNamespaceQuota bool
+	instances           int64
 }
 
 // NewQuotaChecker creates a new quota checker.
 // checkNamespaceQuota specifies whether or not the checker should also take the amount of existing
 // namespaces into account, or not.
-func NewQuotaChecker(c client.Client, claimName, claimNamespace, instanceNamespace string, r utils.Resources, gr schema.GroupResource, gk schema.GroupKind, checkNamespaceQuota bool) QuotaChecker {
+func NewQuotaChecker(c client.Client, claimName, claimNamespace, instanceNamespace string, r utils.Resources, gr schema.GroupResource, gk schema.GroupKind, checkNamespaceQuota bool, instances int64) QuotaChecker {
 	return QuotaChecker{
 		requestedResources:  r,
 		instanceNamespace:   instanceNamespace,
@@ -40,6 +42,7 @@ func NewQuotaChecker(c client.Client, claimName, claimNamespace, instanceNamespa
 		gr:                  gr,
 		gk:                  gk,
 		checkNamespaceQuota: checkNamespaceQuota,
+		instances:           instances,
 	}
 }
 
@@ -60,7 +63,7 @@ func (q *QuotaChecker) CheckQuotas(ctx context.Context) *apierrors.StatusError {
 		return nsErr
 	}
 
-	return q.requestedResources.CheckResourcesAgainstQuotas(ctx, q.client, q.claimName, q.instanceNamespace, q.gk)
+	return q.requestedResources.CheckResourcesAgainstQuotas(ctx, q.client, q.claimName, q.instanceNamespace, q.gk, q.instances)
 }
 
 // areNamespacesWithinQuota checks for the given organization if there are still enough namespaces available.
@@ -148,36 +151,38 @@ func (q *QuotaChecker) getNamespaceOverrides(ctx context.Context, c client.Clien
 
 // AddInitalNamespaceQuotas will add the default quotas to the namespace annotations.
 // It will only add them, if there are currently no such annotations in place.
-func AddInitalNamespaceQuotas(ns *corev1.Namespace) bool {
+func AddInitalNamespaceQuotas(ctx context.Context, iof *iofruntime.Runtime, ns *corev1.Namespace, s *utils.Sidecars, kind string) bool {
 	annotations := ns.GetAnnotations()
 	if annotations == nil {
 		annotations = map[string]string{}
 	}
 
 	added := false
 
+	r := utils.GetDefaultResources(kind, s)
+
 	if _, ok := annotations[utils.DiskAnnotation]; !ok {
 		annotations[utils.DiskAnnotation] = utils.DefaultDiskRequests.String()
 		added = true
 	}
 
 	if _, ok := annotations[utils.CpuRequestAnnotation]; !ok {
-		annotations[utils.CpuRequestAnnotation] = utils.DefaultCPURequests.String()
+		annotations[utils.CpuRequestAnnotation] = r.CPURequests.String()
 		added = true
 	}
 
 	if _, ok := annotations[utils.CpuLimitAnnotation]; !ok {
-		annotations[utils.CpuLimitAnnotation] = utils.DefaultCPULimit.String()
+		annotations[utils.CpuLimitAnnotation] = r.CPULimits.String()
 		added = true
 	}
 
 	if _, ok := annotations[utils.MemoryRequestAnnotation]; !ok {
-		annotations[utils.MemoryRequestAnnotation] = utils.DefaultMemoryRequests.String()
+		annotations[utils.MemoryRequestAnnotation] = r.MemoryRequests.String()
 		added = true
 	}
 
 	if _, ok := annotations[utils.MemoryLimitAnnotation]; !ok {
-		annotations[utils.MemoryLimitAnnotation] = utils.DefaultMemoryLimits.String()
+		annotations[utils.MemoryLimitAnnotation] = r.MemoryLimits.String()
 		added = true
 	}
 

pkg/common/quotas/quotas_test.go

@@ -7,8 +7,10 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/vshn/appcat/v4/apis/metadata"
 	"github.com/vshn/appcat/v4/pkg"
 	"github.com/vshn/appcat/v4/pkg/common/utils"
+	"github.com/vshn/appcat/v4/pkg/comp-functions/functions/commontest"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -38,6 +40,7 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 		instanceNS  *corev1.Namespace
 		dummyNs     int
 		NSOverrides int
+		instances   int64
 	}{
 		{
 			name: "GivenNoInstanceNamespace_WhenCheckingAgainstDefault_ThenNoError",
@@ -48,6 +51,7 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 				MemoryRequests: *resource.NewQuantity(1811939328, resource.BinarySI),
 				MemoryLimits:   *resource.NewQuantity(1811939328, resource.BinarySI),
 			},
+			instances: 1,
 		},
 		{
 			name: "GivenNotInstancenamespace_WhenCheckingAgainstDefault_ThenError",
@@ -58,7 +62,8 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 				MemoryRequests: *resource.NewQuantity(1811939328, resource.BinarySI),
 				MemoryLimits:   *resource.NewQuantity(1811939328, resource.BinarySI),
 			},
-			wantErr: true,
+			instances: 1,
+			wantErr:   true,
 		},
 		{
 			name: "GivenInstancenamespace_WhenCheckingAgainst_ThenNoError",
@@ -77,6 +82,7 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 					},
 				},
 			},
+			instances: 1,
 		},
 		{
 			name: "GivenInstancenamespace_WhenCheckingAgainst_ThenError",
@@ -95,7 +101,8 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 					},
 				},
 			},
-			wantErr: true,
+			instances: 1,
+			wantErr:   true,
 		},
 		{
 			name: "GivenTooManyNamespaces_ThenError",
@@ -106,8 +113,9 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 				MemoryRequests: *resource.NewQuantity(1811939328, resource.BinarySI),
 				MemoryLimits:   *resource.NewQuantity(1811939328, resource.BinarySI),
 			},
-			wantErr: true,
-			dummyNs: 30,
+			instances: 1,
+			wantErr:   true,
+			dummyNs:   30,
 		},
 		{
 			name: "GivenNamespaceOverride_ThenNoError",
@@ -118,6 +126,7 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 				MemoryRequests: *resource.NewQuantity(1811939328, resource.BinarySI),
 				MemoryLimits:   *resource.NewQuantity(1811939328, resource.BinarySI),
 			},
+			instances:   1,
 			dummyNs:     30,
 			NSOverrides: 35,
 		},
@@ -130,8 +139,16 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 				WithScheme(pkg.SetupScheme()).
 				WithObjects(ns).Build()
 
+			iof := commontest.LoadRuntimeFromFile(t, "common/quotas/01_default.yaml")
+			objectMeta := &metadata.MetadataOnlyObject{}
+
+			err := iof.Desired.GetComposite(ctx, objectMeta)
+			assert.NoError(t, err)
+
 			if tt.instanceNS != nil {
-				AddInitalNamespaceQuotas(ns)
+				s := &utils.Sidecars{}
+				assert.NoError(t, err)
+				AddInitalNamespaceQuotas(ctx, iof, ns, s, gk.Kind)
 				assert.NoError(t, fclient.Create(ctx, tt.instanceNS))
 			}
 
@@ -165,8 +182,8 @@ func TestQuotaChecker_CheckQuotas(t *testing.T) {
 			}
 
 			//when
-			checker := NewQuotaChecker(fclient, "mytest", "myns", "testns", tt.requested, gr, gk, true)
-			err := checker.CheckQuotas(ctx)
+			checker := NewQuotaChecker(fclient, "mytest", "myns", "testns", tt.requested, gr, gk, true, tt.instances)
+			err = checker.CheckQuotas(ctx)
 
 			//Then
 			// Unfortunately assert.Error and assert.NoError give us false positives here.

pkg/common/utils/resources.go

@@ -3,6 +3,7 @@ package utils
 import (
 	"context"
 	"fmt"
+	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -76,7 +77,7 @@ var (
 	// defaultCPURequests 2* standard-8 will request 4 CPUs. This default has 500m as spare for jobs
 	DefaultCPURequests = resource.NewMilliQuantity(4500, resource.DecimalSI)
 	// defaultCPULimit by default same as DefaultCPURequests
-	DefaultCPULimit = DefaultCPURequests
+	DefaultCPULimits = DefaultCPURequests
 	// defaultMemoryRequests 2* standard-8 will request 16Gb. This default has 500mb as spare for jobs
 	DefaultMemoryRequests = resource.NewQuantity(17301504000, resource.BinarySI)
 	// defaultMemoryLimits same as DefaultMemoryRequests
@@ -91,9 +92,10 @@ var (
 // The second case is usually triggered if a new instance is created, as we don't have a
 // namespace to check against.
 // Once the namespace exists, the composition should ensure that the annotations are set.
-func (r *Resources) CheckResourcesAgainstQuotas(ctx context.Context, c client.Client, claimName, instanceNamespace string, gk schema.GroupKind) *apierrors.StatusError {
+func (r *Resources) CheckResourcesAgainstQuotas(ctx context.Context, c client.Client, claimName, instanceNamespace string, gk schema.GroupKind, instances int64) *apierrors.StatusError {
 
-	nsQuotas := Resources{}
+	var nsQuotas Resources
+	s := &Sidecars{}
 	if instanceNamespace != "" {
 		q, err := r.getNamespaceQuotas(ctx, c, instanceNamespace)
 		if err != nil {
@@ -102,9 +104,18 @@ func (r *Resources) CheckResourcesAgainstQuotas(ctx context.Context, c client.Cl
 			}
 		}
 		nsQuotas = q
+	} else {
+		if gk.Kind == "VSHNPostgreSQL" {
+			var err error
+			s, err = FetchSidecarsFromCluster(ctx, c, "vshnpostgresqlplans")
+			if err != nil {
+				return apierrors.NewInternalError(err)
+			}
+		}
+		nsQuotas = *GetDefaultResources(gk.Kind, s)
 	}
 
-	quotaErrs := r.checkAgainstResources(nsQuotas)
+	quotaErrs := r.checkAgainstResources(nsQuotas, gk.Kind, s)
 
 	if len(quotaErrs) == 0 {
 		return nil
@@ -181,46 +192,49 @@ func (r *Resources) getNamespaceQuotas(ctx context.Context, c client.Client, ins
 
 // checkAgainstResources compares this resources against the given resources.
 // Any non-populated fields are checked against their defaults.
-func (r *Resources) checkAgainstResources(quotaResources Resources) field.ErrorList {
+func (r *Resources) checkAgainstResources(quotaResources Resources, kind string, s *Sidecars) field.ErrorList {
 	foundErrs := field.ErrorList{}
 
-	errCPURequests := field.Forbidden(r.CPURequestsPath, "Max allowed CPU requests: "+DefaultCPURequests.String()+". Configured requests: "+r.CPURequests.String()+". "+contactSupportMessage)
+	rDef := GetDefaultResources(kind, s)
+
+	errCPURequests := field.Forbidden(r.CPURequestsPath, "Max allowed CPU requests: "+quotaResources.CPURequests.String()+". Configured requests: "+r.CPURequests.String()+". "+contactSupportMessage)
+
 	if !quotaResources.CPURequests.IsZero() {
 		if r.CPURequests.Cmp(quotaResources.CPURequests) == 1 {
 			foundErrs = append(foundErrs, errCPURequests)
 		}
-	} else if r.CPURequests.Cmp(*DefaultCPURequests) == 1 {
+	} else if r.CPURequests.Cmp(rDef.CPURequests) == 1 {
 		foundErrs = append(foundErrs, errCPURequests)
 	}
 
-	errCPULimits := field.Forbidden(r.CPULimitsPath, "Max allowed CPU limits: "+DefaultCPULimit.String()+". Configured limits: "+r.CPULimits.String()+". "+contactSupportMessage)
+	errCPULimits := field.Forbidden(r.CPULimitsPath, "Max allowed CPU limits: "+quotaResources.CPULimits.String()+". Configured limits: "+r.CPULimits.String()+". "+contactSupportMessage)
 	if !quotaResources.CPULimits.IsZero() {
 		if r.CPULimits.Cmp(quotaResources.CPULimits) == 1 {
 			foundErrs = append(foundErrs, errCPULimits)
 		}
-	} else if r.CPULimits.Cmp(*DefaultCPULimit) == 1 {
+	} else if r.CPULimits.Cmp(rDef.CPULimits) == 1 {
 		foundErrs = append(foundErrs, errCPULimits)
 	}
 
-	errMemoryRequests := field.Forbidden(r.MemoryRequestsPath, "Max allowed Memory requests: "+DefaultMemoryRequests.String()+". Configured requests: "+r.MemoryRequests.String()+". "+contactSupportMessage)
+	errMemoryRequests := field.Forbidden(r.MemoryRequestsPath, "Max allowed Memory requests: "+quotaResources.MemoryRequests.String()+". Configured requests: "+r.MemoryRequests.String()+". "+contactSupportMessage)
 	if !quotaResources.MemoryRequests.IsZero() {
 		if r.MemoryRequests.Cmp(quotaResources.MemoryRequests) == 1 {
 			foundErrs = append(foundErrs, errMemoryRequests)
 		}
-	} else if r.MemoryRequests.Cmp(*DefaultMemoryRequests) == 1 {
+	} else if r.MemoryRequests.Cmp(rDef.MemoryRequests) == 1 {
 		foundErrs = append(foundErrs, errMemoryRequests)
 	}
 
-	errMemoryLimits := field.Forbidden(r.MemoryLimitsPath, "Max allowed Memory limits: "+DefaultMemoryLimits.String()+". Configured limits: "+r.MemoryLimits.String()+". "+contactSupportMessage)
+	errMemoryLimits := field.Forbidden(r.MemoryLimitsPath, "Max allowed Memory limits: "+quotaResources.MemoryLimits.String()+". Configured limits: "+r.MemoryLimits.String()+". "+contactSupportMessage)
 	if !quotaResources.MemoryLimits.IsZero() {
 		if r.MemoryLimits.Cmp(quotaResources.MemoryLimits) == 1 {
 			foundErrs = append(foundErrs, errMemoryLimits)
 		}
-	} else if r.MemoryLimits.Cmp(*DefaultMemoryLimits) == 1 {
+	} else if r.MemoryLimits.Cmp(rDef.MemoryLimits) == 1 {
 		foundErrs = append(foundErrs, errMemoryLimits)
 	}
 
-	errDisk := field.Forbidden(r.DiskPath, "Max allowed Disk: "+DefaultCPULimit.String()+". Configured requests: "+r.Disk.String()+". "+contactSupportMessage)
+	errDisk := field.Forbidden(r.DiskPath, "Max allowed Disk: "+quotaResources.Disk.String()+". Configured requests: "+r.Disk.String()+". "+contactSupportMessage)
 	if !quotaResources.Disk.IsZero() {
 		if r.Disk.Cmp(quotaResources.Disk) == 1 {
 			foundErrs = append(foundErrs, errDisk)
@@ -248,3 +262,39 @@ func (r *Resources) MultiplyBy(i int64) {
 	r.MemoryRequests.Set(r.MemoryRequests.Value() * i)
 	r.Disk.Set(r.Disk.Value() * i)
 }
+
+func (r *Resources) AddResources(resource Resources) {
+	r.CPULimits.Add(resource.CPULimits)
+	r.CPURequests.Add(resource.CPURequests)
+	r.MemoryLimits.Add(resource.MemoryLimits)
+	r.MemoryRequests.Add(resource.MemoryRequests)
+	r.Disk.Add(resource.Disk)
+}
+
+// AddPsqlSidecarResources adds the resource overhead for the PostgreSQL sidecar to the given resource.
+func (r *Resources) AddPsqlSidecarResources(s *Sidecars, instances int64) {
+	resourcesSidecars, err := GetAllSideCarsResources(s)
+	if err != nil {
+		instances = 1
+	}
+	r.CPURequests.Add(*resource.NewMilliQuantity(resourcesSidecars.CPURequests.MilliValue()*instances, resource.DecimalSI))
+	r.MemoryRequests.Add(*resource.NewQuantity(resourcesSidecars.MemoryRequests.Value()*instances, resource.BinarySI))
+
+	// Sidecar limits: 4800m CPU, 6144Mi Memory
+	r.CPULimits.Add(*resource.NewMilliQuantity(resourcesSidecars.CPULimits.MilliValue()*instances, resource.DecimalSI))
+	r.MemoryLimits.Add(*resource.NewQuantity(resourcesSidecars.MemoryLimits.Value()*instances, resource.BinarySI))
+}
+
+// GetDefaultResources returns a new Resources struct with the default values.
+func GetDefaultResources(kind string, s *Sidecars) *Resources {
+	r := &Resources{
+		CPURequests:    *DefaultCPURequests,
+		CPULimits:      *DefaultCPULimits,
+		MemoryRequests: *DefaultMemoryRequests,
+		MemoryLimits:   *DefaultMemoryLimits,
+	}
+	if strings.Contains(kind, "VSHNPostgreSQL") {
+		r.AddPsqlSidecarResources(s, int64(2))
+	}
+	return r
+}