diff --git a/components/function-controller/design/scaling_functions.md b/components/function-controller/design/scaling_functions.md new file mode 100644 index 000000000000..a49b08e00c66 --- /dev/null +++ b/components/function-controller/design/scaling_functions.md @@ -0,0 +1,40 @@ +# Serverless Functions Scaling Modes + +## Summary +Initially, the Serverless API supported the `spec.ScaleConfig` field only. The workflow was as follows: +- Function resources were defaulted to min/max replicas = 1 +- If you set a different scaling config, Function Controller created HPA resources with the user-defined min/max values. +- The HPA resource target ref was the Function runtime deployment. +- The Function Controller did not enforce the runtime deployment `spec.Replicas` anymore and it was handled by the HPA resources. + + +Later, the Serverless API was extended by the `Scale` subresource, which allows for direct scaling of the Function resources through the Kubernetes API. + +However, there are some implementation conflicts between the two features. This is a design and an implementation plan to unify the UX while using Functions as scaled resources. + +### Goals +- Support Function scale subresource and `spec.ScaleConfig` without conflicts. +- Provide frictionless UX for the feature. + +## Proposal +Describe and implement two different scaling configuration. Both configurations already work to some extent. The point here is have an ergonomic UX and flow. + +### External scaling configuration +This is managed and configured using `spec.Replicas`. It describes the scale subresource use case. It supports: +- Manual scaling of the Function up and down through the API. +- Configuring an HPA resource with the Function resources as a target. +- Using an external scaler like [KEDA](https://keda.sh/). + +### Built-in scaling configuration +This is managed and enabled by setting `spec.ScaleConfig`. It is configurable using Busola and it provides the most basic scaling configuration for the Function. + +This configuration is disabled by removing `spec.ScaleConfig`. The Busola UI can be extended to allow you to add or remove `spec.ScaleConfig` to manage built-in scaling with minimal effort. + +## Implementation details + +- The controller should support and accept both `spec.Replicas` and `spec.ScaleConfig`. Current validation rule to block this will be removed. +- `spec.Replicas` is the only source of truth for scaling the Function/runtime deployment. +- `spec.ScaleConfig` is only used to configure the controller internal HPA. +- The internal HPA is removed if `spec.ScaleConfig == nil` +- The HPA resource created by the controller still targets the Function resources. +- The current Function status update logic must be fixed to reflect the Function's current scale. \ No newline at end of file diff --git a/components/function-controller/internal/controllers/serverless/deployment.go b/components/function-controller/internal/controllers/serverless/deployment.go index 9e58b1e6ddfe..12ac0923ffb9 100644 --- a/components/function-controller/internal/controllers/serverless/deployment.go +++ b/components/function-controller/internal/controllers/serverless/deployment.go @@ -46,7 +46,6 @@ func stateFnCheckDeployments(ctx context.Context, r *reconciler, s *systemState) } expectedDeployment := s.buildDeployment(args) - deploymentChanged := !s.deploymentEqual(expectedDeployment) if !deploymentChanged { @@ -61,7 +60,7 @@ func stateFnCheckDeployments(ctx context.Context, r *reconciler, s *systemState) return stateFnDeleteDeployments } - if !equalDeployments(s.deployments.Items[0], expectedDeployment, isScalingEnabled(&s.instance)) { + if !equalDeployments(s.deployments.Items[0], expectedDeployment) { return buildStateFnUpdateDeployment(expectedDeployment.Spec, expectedDeployment.Labels) } diff --git a/components/function-controller/internal/controllers/serverless/deployment_test.go b/components/function-controller/internal/controllers/serverless/deployment_test.go index 843c9838e642..ceffaca1694e 100644 --- a/components/function-controller/internal/controllers/serverless/deployment_test.go +++ b/components/function-controller/internal/controllers/serverless/deployment_test.go @@ -332,7 +332,7 @@ func TestFunctionReconciler_equalDeployments(t *testing.T) { expected: fixDeploymentWithReplicas(2), scalingEnabled: true, }, - want: true, + want: false, }, { name: "scaling enabled and replicas match", @@ -365,7 +365,7 @@ func TestFunctionReconciler_equalDeployments(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := gomega.NewGomegaWithT(t) - got := equalDeployments(tt.args.existing, tt.args.expected, tt.args.scalingEnabled) + got := equalDeployments(tt.args.existing, tt.args.expected) g.Expect(got).To(gomega.Equal(tt.want)) }) } diff --git a/components/function-controller/internal/controllers/serverless/function_reconcile_asserts_test.go b/components/function-controller/internal/controllers/serverless/function_reconcile_asserts_test.go index 2b5400e72bdf..ed0dd8d59042 100644 --- a/components/function-controller/internal/controllers/serverless/function_reconcile_asserts_test.go +++ b/components/function-controller/internal/controllers/serverless/function_reconcile_asserts_test.go @@ -192,9 +192,9 @@ func assertSuccessfulFunctionDeployment(t *testing.T, resourceClient resource.Cl hpaSpec := hpaList.Items[0].Spec - g.Expect(hpaSpec.ScaleTargetRef.Name).To(gomega.Equal(deployment.GetName())) - g.Expect(hpaSpec.ScaleTargetRef.Kind).To(gomega.Equal("Deployment")) - g.Expect(hpaSpec.ScaleTargetRef.APIVersion).To(gomega.Equal(appsv1.SchemeGroupVersion.String())) + g.Expect(hpaSpec.ScaleTargetRef.Name).To(gomega.Equal(function.GetName())) + g.Expect(hpaSpec.ScaleTargetRef.Kind).To(gomega.Equal(serverlessv1alpha2.FunctionKind)) + g.Expect(hpaSpec.ScaleTargetRef.APIVersion).To(gomega.Equal(serverlessv1alpha2.GroupVersion.String())) t.Log("deployment ready") deployment.Status.Conditions = []appsv1.DeploymentCondition{ diff --git a/components/function-controller/internal/controllers/serverless/function_reconcile_gitops_test.go b/components/function-controller/internal/controllers/serverless/function_reconcile_gitops_test.go index e6cdf01c53e1..45c20ffb84e6 100644 --- a/components/function-controller/internal/controllers/serverless/function_reconcile_gitops_test.go +++ b/components/function-controller/internal/controllers/serverless/function_reconcile_gitops_test.go @@ -357,7 +357,7 @@ func TestGitOpsWithContinuousGitCheckout(t *testing.T) { g.Expect(svc.Spec.Selector).To(gomega.Equal(deployment.Spec.Selector.MatchLabels)) - t.Log("hpa creation") + t.Log("HPA creation") g.Expect(reconciler.Reconcile(ctx, request)).To(beOKReconcileResult) function = &serverlessv1alpha2.Function{} @@ -375,9 +375,9 @@ func TestGitOpsWithContinuousGitCheckout(t *testing.T) { hpaSpec := hpaList.Items[0].Spec - g.Expect(hpaSpec.ScaleTargetRef.Name).To(gomega.Equal(deployment.GetName())) - g.Expect(hpaSpec.ScaleTargetRef.Kind).To(gomega.Equal("Deployment")) - g.Expect(hpaSpec.ScaleTargetRef.APIVersion).To(gomega.Equal(appsv1.SchemeGroupVersion.String())) + g.Expect(hpaSpec.ScaleTargetRef.Name).To(gomega.Equal(function.GetName())) + g.Expect(hpaSpec.ScaleTargetRef.Kind).To(gomega.Equal(serverlessv1alpha2.FunctionKind)) + g.Expect(hpaSpec.ScaleTargetRef.APIVersion).To(gomega.Equal(serverlessv1alpha2.GroupVersion.String())) t.Log("deployment ready") deployment.Status.Conditions = []appsv1.DeploymentCondition{ @@ -650,9 +650,9 @@ func TestGitOpsWithoutContinuousGitCheckout(t *testing.T) { hpaSpec := hpaList.Items[0].Spec - g.Expect(hpaSpec.ScaleTargetRef.Name).To(gomega.Equal(deployment.GetName())) - g.Expect(hpaSpec.ScaleTargetRef.Kind).To(gomega.Equal("Deployment")) - g.Expect(hpaSpec.ScaleTargetRef.APIVersion).To(gomega.Equal(appsv1.SchemeGroupVersion.String())) + g.Expect(hpaSpec.ScaleTargetRef.Name).To(gomega.Equal(function.GetName())) + g.Expect(hpaSpec.ScaleTargetRef.Kind).To(gomega.Equal(serverlessv1alpha2.FunctionKind)) + g.Expect(hpaSpec.ScaleTargetRef.APIVersion).To(gomega.Equal(serverlessv1alpha2.GroupVersion.String())) t.Log("Deployment is ready") deployment.Status.Conditions = []appsv1.DeploymentCondition{ diff --git a/components/function-controller/internal/controllers/serverless/function_reconcile_test.go b/components/function-controller/internal/controllers/serverless/function_reconcile_test.go index 0259a1521086..1b5a9e8f8786 100644 --- a/components/function-controller/internal/controllers/serverless/function_reconcile_test.go +++ b/components/function-controller/internal/controllers/serverless/function_reconcile_test.go @@ -96,14 +96,15 @@ func TestFunctionReconciler_Reconcile_Scaling(t *testing.T) { assertSuccessfulFunctionBuild(t, resourceClient, reconciler, request, fnLabels, false) assertSuccessfulFunctionDeployment(t, resourceClient, reconciler, request, fnLabels, "registry.kyma.local", false) - two := int32(2) four := int32(4) t.Log("updating function to use fixed replicas number") g.Expect(resourceClient.Get(context.TODO(), request.NamespacedName, function)).To(gomega.Succeed()) - function.Spec.ScaleConfig.MinReplicas = &two function.Spec.ScaleConfig.MaxReplicas = &two + function.Spec.ScaleConfig.MinReplicas = &two + // TODO: This should be applied by the defaulting webhook + function.Spec.Replicas = &two g.Expect(resourceClient.Update(context.TODO(), function)).To(gomega.Succeed()) t.Log("updating deployment with new number of replicas") @@ -125,7 +126,7 @@ func TestFunctionReconciler_Reconcile_Scaling(t *testing.T) { g.Expect(deployment).ToNot(gomega.BeNil()) g.Expect(deployment.Spec.Replicas).To(gomega.Equal(&two)) - t.Log("removing hpa") + t.Log("HPA is removed") result, err = reconciler.Reconcile(ctx, request) g.Expect(err).To(gomega.BeNil()) g.Expect(result.Requeue).To(gomega.BeFalse()) @@ -149,12 +150,12 @@ func TestFunctionReconciler_Reconcile_Scaling(t *testing.T) { g.Expect(getConditionStatus(function.Status.Conditions, serverlessv1alpha2.ConditionRunning)).To(gomega.Equal(corev1.ConditionTrue)) g.Expect(getConditionReason(function.Status.Conditions, serverlessv1alpha2.ConditionRunning)).To(gomega.Equal(serverlessv1alpha2.ConditionReasonDeploymentReady)) - t.Log("updating function to use scalable replicas number") + t.Log("replicas increased by an external scaler") g.Expect(resourceClient.Get(context.TODO(), request.NamespacedName, function)).To(gomega.Succeed()) - function.Spec.ScaleConfig.MaxReplicas = &four + function.Spec.Replicas = &four g.Expect(resourceClient.Update(context.TODO(), function)).To(gomega.Succeed()) - t.Log("creating hpa") + t.Log("Updating deployment") result, err = reconciler.Reconcile(ctx, request) g.Expect(err).To(gomega.BeNil()) g.Expect(result.Requeue).To(gomega.BeFalse()) @@ -166,19 +167,17 @@ func TestFunctionReconciler_Reconcile_Scaling(t *testing.T) { g.Expect(getConditionStatus(function.Status.Conditions, serverlessv1alpha2.ConditionBuildReady)).To(gomega.Equal(corev1.ConditionTrue)) g.Expect(getConditionStatus(function.Status.Conditions, serverlessv1alpha2.ConditionRunning)).To(gomega.Equal(corev1.ConditionUnknown)) - g.Expect(getConditionReason(function.Status.Conditions, serverlessv1alpha2.ConditionRunning)).To(gomega.Equal(serverlessv1alpha2.ConditionReasonHorizontalPodAutoscalerCreated)) + g.Expect(getConditionReason(function.Status.Conditions, serverlessv1alpha2.ConditionRunning)).To(gomega.Equal(serverlessv1alpha2.ConditionReasonDeploymentUpdated)) + // we scale the deployment directly using spec.Replicas, a new HPA shouldn't be created err = reconciler.client.ListByLabel(context.TODO(), function.GetNamespace(), fnLabels, hpaList) g.Expect(err).To(gomega.BeNil()) - g.Expect(hpaList.Items).To(gomega.HaveLen(1)) - - hpaSpec := hpaList.Items[0].Spec - - g.Expect(hpaSpec.ScaleTargetRef.Name).To(gomega.Equal(deployment.GetName())) - g.Expect(hpaSpec.ScaleTargetRef.Kind).To(gomega.Equal("Deployment")) - g.Expect(hpaSpec.ScaleTargetRef.APIVersion).To(gomega.Equal(appsv1.SchemeGroupVersion.String())) + g.Expect(hpaList.Items).To(gomega.HaveLen(0)) t.Log("deployment ready") + g.Expect(resourceClient.ListByLabel(context.TODO(), request.Namespace, fnLabels, deployments)).To(gomega.Succeed()) + g.Expect(len(deployments.Items)).To(gomega.Equal(1)) + deployment = &deployments.Items[0] deployment.Status.Conditions = []appsv1.DeploymentCondition{ {Type: appsv1.DeploymentAvailable, Status: corev1.ConditionTrue, Reason: MinimumReplicasAvailable}, {Type: appsv1.DeploymentProgressing, Status: corev1.ConditionTrue, Reason: NewRSAvailableReason}, @@ -705,7 +704,7 @@ func TestFunctionReconciler_Reconcile(t *testing.T) { _, err = reconciler.Reconcile(ctx, request) g.Expect(err).To(gomega.BeNil()) - g.Expect(hpaList.Items[0].Spec.ScaleTargetRef.Name).To(gomega.Equal(deployList.Items[0].Name), "hpa should target proper deployment") + g.Expect(hpaList.Items[0].Spec.ScaleTargetRef.Name).To(gomega.Equal(inFunction.GetName()), "hpa should target the function") t.Log("deleting deployment by 'accident' to check proper hpa-deployment reference") @@ -731,7 +730,7 @@ func TestFunctionReconciler_Reconcile(t *testing.T) { g.Expect(err).To(gomega.BeNil()) g.Expect(deployList.Items).To(gomega.HaveLen(1)) - g.Expect(hpaList.Items[0].Spec.ScaleTargetRef.Name).To(gomega.Equal(deployList.Items[0].Name), "hpa should target proper deployment") + g.Expect(hpaList.Items[0].Spec.ScaleTargetRef.Name).To(gomega.Equal(function.GetName()), "hpa should target function") }) t.Run("should requeue before creating a job", func(t *testing.T) { diff --git a/components/function-controller/internal/controllers/serverless/scaling.go b/components/function-controller/internal/controllers/serverless/scaling.go index 62ac9ad587c4..aed4bc043330 100644 --- a/components/function-controller/internal/controllers/serverless/scaling.go +++ b/components/function-controller/internal/controllers/serverless/scaling.go @@ -20,7 +20,7 @@ func stateFnCheckScaling(ctx context.Context, r *reconciler, s *systemState) sta return nil } - if s.instance.Spec.Replicas != nil { + if !isScalingEnabled(&s.instance) { return stateFnCheckReplicas(ctx, r, s) } @@ -56,7 +56,7 @@ func stateFnCheckHPA(ctx context.Context, r *reconciler, s *systemState) stateFn return stateFnDeleteAllHorizontalPodAutoscalers } - if numHpa == 1 && equalInt32Pointer(s.instance.Spec.ScaleConfig.MinReplicas, s.instance.Spec.ScaleConfig.MaxReplicas) { + if numHpa == 1 && !isScalingEnabled(&s.instance) { // this case is when we previously created HPA with maxReplicas > minReplicas, but now user changed // function spec and NOW maxReplicas == minReplicas, so hpa is not needed anymore return stateFnDeleteAllHorizontalPodAutoscalers diff --git a/components/function-controller/internal/controllers/serverless/system_state.go b/components/function-controller/internal/controllers/serverless/system_state.go index e64c0336ad9e..3116710bb3ce 100644 --- a/components/function-controller/internal/controllers/serverless/system_state.go +++ b/components/function-controller/internal/controllers/serverless/system_state.go @@ -495,20 +495,16 @@ func (s *systemState) buildDeployment(cfg buildDeploymentArgs) appsv1.Deployment } func (s *systemState) getReplicas(defaultVal int32) *int32 { - if s.instance.Spec.ScaleConfig != nil { - return s.instance.Spec.ScaleConfig.MinReplicas - } if s.instance.Spec.Replicas != nil { return s.instance.Spec.Replicas } - return &defaultVal } // TODO do not negate func (s *systemState) deploymentEqual(d appsv1.Deployment) bool { return len(s.deployments.Items) == 1 && - equalDeployments(s.deployments.Items[0], d, isScalingEnabled(&s.instance)) + equalDeployments(s.deployments.Items[0], d) } func (s *systemState) hasDeploymentConditionTrueStatusWithReason(conditionType appsv1.DeploymentConditionType, reason string) bool { @@ -604,7 +600,6 @@ func (s *systemState) defaultReplicas() (int32, int32) { func (s *systemState) buildHorizontalPodAutoscaler(targetCPUUtilizationPercentage int32) autoscalingv1.HorizontalPodAutoscaler { minReplicas, maxReplicas := s.defaultReplicas() - deploymentName := s.deployments.Items[0].GetName() return autoscalingv1.HorizontalPodAutoscaler{ ObjectMeta: metav1.ObjectMeta{ GenerateName: fmt.Sprintf("%s-", s.instance.GetName()), @@ -613,9 +608,9 @@ func (s *systemState) buildHorizontalPodAutoscaler(targetCPUUtilizationPercentag }, Spec: autoscalingv1.HorizontalPodAutoscalerSpec{ ScaleTargetRef: autoscalingv1.CrossVersionObjectReference{ - Kind: "Deployment", - Name: deploymentName, - APIVersion: appsv1.SchemeGroupVersion.String(), + Kind: serverlessv1alpha2.FunctionKind, + Name: s.instance.Name, + APIVersion: serverlessv1alpha2.GroupVersion.String(), }, MinReplicas: &minReplicas, MaxReplicas: maxReplicas, diff --git a/components/function-controller/internal/controllers/serverless/utils.go b/components/function-controller/internal/controllers/serverless/utils.go index f35da111f9fe..2fa6674c61f6 100644 --- a/components/function-controller/internal/controllers/serverless/utils.go +++ b/components/function-controller/internal/controllers/serverless/utils.go @@ -219,7 +219,7 @@ func mapsEqual(existing, expected map[string]string) bool { } // TODO refactor to make this code more readable -func equalDeployments(existing appsv1.Deployment, expected appsv1.Deployment, scalingEnabled bool) bool { +func equalDeployments(existing appsv1.Deployment, expected appsv1.Deployment) bool { return len(existing.Spec.Template.Spec.Containers) == 1 && len(existing.Spec.Template.Spec.Containers) == len(expected.Spec.Template.Spec.Containers) && existing.Spec.Template.Spec.Containers[0].Image == expected.Spec.Template.Spec.Containers[0].Image && @@ -227,7 +227,7 @@ func equalDeployments(existing appsv1.Deployment, expected appsv1.Deployment, sc mapsEqual(existing.GetLabels(), expected.GetLabels()) && mapsEqual(existing.Spec.Template.GetLabels(), expected.Spec.Template.GetLabels()) && equalResources(existing.Spec.Template.Spec.Containers[0].Resources, expected.Spec.Template.Spec.Containers[0].Resources) && - (scalingEnabled || equalInt32Pointer(existing.Spec.Replicas, expected.Spec.Replicas)) + equalInt32Pointer(existing.Spec.Replicas, expected.Spec.Replicas) } func equalServices(existing corev1.Service, expected corev1.Service) bool { diff --git a/components/function-controller/internal/webhook/defaulting_webhook.go b/components/function-controller/internal/webhook/defaulting_webhook.go index fdca9c36bc82..b9a65a284816 100644 --- a/components/function-controller/internal/webhook/defaulting_webhook.go +++ b/components/function-controller/internal/webhook/defaulting_webhook.go @@ -66,7 +66,6 @@ func (w *DefaultingWebHook) handleFunctionDefaulting(req admission.Request) admi } fn.Default(w.configAlphaV2) f = fn - } default: return admission.Errored(http.StatusBadRequest, errors.Errorf("Invalid resource version provided: %s", req.Kind.Version)) diff --git a/components/function-controller/pkg/apis/serverless/v1alpha2/function_defaults.go b/components/function-controller/pkg/apis/serverless/v1alpha2/function_defaults.go index 7d9bd4dfae46..83a375021fb2 100644 --- a/components/function-controller/pkg/apis/serverless/v1alpha2/function_defaults.go +++ b/components/function-controller/pkg/apis/serverless/v1alpha2/function_defaults.go @@ -58,30 +58,34 @@ type DefaultingConfig struct { } func (fn *Function) Default(config *DefaultingConfig) { - fn.Spec.defaultReplicas(config) + fn.Spec.defaultScaling(config) fn.Spec.defaultFunctionResources(config, fn) fn.Spec.defaultBuildResources(config, fn) } -func (spec *FunctionSpec) defaultReplicas(config *DefaultingConfig) { - if spec.Replicas != nil { - return - } - +func (spec *FunctionSpec) defaultScaling(config *DefaultingConfig) { defaultingConfig := config.Function.Replicas replicasPreset := defaultingConfig.Presets[defaultingConfig.DefaultPreset] + if spec.Replicas == nil { + // TODO: The presets structure and docs should be updated to reflect the new behavior. + spec.Replicas = &replicasPreset.Min + } + if spec.ScaleConfig == nil { - spec.ScaleConfig = &ScaleConfig{} + return } + + // spec.ScaleConfig is SET, but not fully configured, for sanity, we will default MinReplicas, we will also use it as a default spec.Replica if spec.ScaleConfig.MinReplicas == nil { newMin := replicasPreset.Min if spec.ScaleConfig.MaxReplicas != nil && *spec.ScaleConfig.MaxReplicas < newMin { newMin = *spec.ScaleConfig.MaxReplicas } - spec.ScaleConfig.MinReplicas = &newMin } + spec.Replicas = spec.ScaleConfig.MinReplicas + if spec.ScaleConfig.MaxReplicas == nil { newMax := replicasPreset.Max if *spec.ScaleConfig.MinReplicas > newMax { diff --git a/components/function-controller/pkg/apis/serverless/v1alpha2/function_defaults_test.go b/components/function-controller/pkg/apis/serverless/v1alpha2/function_defaults_test.go index 3c7b8ea55644..9a7208ca6895 100644 --- a/components/function-controller/pkg/apis/serverless/v1alpha2/function_defaults_test.go +++ b/components/function-controller/pkg/apis/serverless/v1alpha2/function_defaults_test.go @@ -82,6 +82,8 @@ func TestSetDefaults(t *testing.T) { MinReplicas: &two, MaxReplicas: &two, }, + + Replicas: &two, }, }, }, @@ -110,6 +112,7 @@ func TestSetDefaults(t *testing.T) { MinReplicas: &two, MaxReplicas: &two, }, + Replicas: &two, }, }, }, @@ -136,6 +139,7 @@ func TestSetDefaults(t *testing.T) { MinReplicas: &two, MaxReplicas: &two, }, + Replicas: &two, }, }, }, @@ -146,10 +150,7 @@ func TestSetDefaults(t *testing.T) { ResourceConfiguration: &ResourceConfiguration{ Function: ResourceRequirementsBuilder{}.Limits("100m", "128Mi").Requests("50m", "64Mi").Build(), }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &one, - MaxReplicas: &one, - }, + Replicas: &one, }, }, }, @@ -195,6 +196,7 @@ func TestSetDefaults(t *testing.T) { MinReplicas: &zero, MaxReplicas: &zero, }, + Replicas: &zero, }, }, }, @@ -254,10 +256,7 @@ func TestSetDefaults(t *testing.T) { Function: ResourceRequirementsBuilder{}.Limits("50m", "64Mi").Requests("25m", "32Mi").Build(), Build: ResourceRequirementsBuilder{}.Limits("700m", "700Mi").Requests("350m", "350Mi").Build(), }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &one, - MaxReplicas: &one, - }, + Replicas: &one, }, }, }, @@ -283,10 +282,7 @@ func TestSetDefaults(t *testing.T) { Function: ResourceRequirementsBuilder{}.Limits("50m", "64Mi").Requests("25m", "32Mi").Profile("S").Build(), Build: ResourceRequirementsBuilder{}.Limits("700m", "700Mi").Requests("350m", "350Mi").Profile("slow").Build(), }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &one, - MaxReplicas: &one, - }, + Replicas: &one, }, }, }, @@ -321,6 +317,7 @@ func TestSetDefaults(t *testing.T) { Function: ResourceRequirementsBuilder{}.Limits("50m", "64Mi").Requests("15m", "15Mi").Build(), Build: ResourceRequirementsBuilder{}.Limits("700m", "700Mi").Requests("250m", "250Mi").Build(), }, + Replicas: &two, ScaleConfig: &ScaleConfig{ MinReplicas: &two, MaxReplicas: &two, @@ -337,9 +334,7 @@ func TestSetDefaults(t *testing.T) { Function: ResourceRequirementsBuilder{}.Requests("15m", "15Mi").Profile("S").Build(), Build: ResourceRequirementsBuilder{}.Requests("250m", "250Mi").Profile("slow").Build(), }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &two, - }, + Replicas: &two, }, }, expectedFunc: Function{ @@ -350,10 +345,7 @@ func TestSetDefaults(t *testing.T) { Function: ResourceRequirementsBuilder{}.Limits("50m", "64Mi").Requests("15m", "15Mi").Profile("S").Build(), Build: ResourceRequirementsBuilder{}.Limits("700m", "700Mi").Requests("250m", "250Mi").Profile("slow").Build(), }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &two, - MaxReplicas: &two, - }, + Replicas: &two, }, }, }, @@ -386,10 +378,7 @@ func TestSetDefaults(t *testing.T) { Resources: &fastBuildResources, }, }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &one, - MaxReplicas: &one, - }, + Replicas: &one, }, }, }, @@ -418,10 +407,7 @@ func TestSetDefaults(t *testing.T) { Resources: &fastBuildResources, }, }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &one, - MaxReplicas: &one, - }, + Replicas: &one, }, }, }, @@ -449,10 +435,7 @@ func TestSetDefaults(t *testing.T) { Resources: &MRuntimeResources, }, }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &one, - MaxReplicas: &one, - }, + Replicas: &one, }}, }, "Should set function profile to function presets M instead of default L value (using ResourceConfiguration..Preset)": { @@ -475,10 +458,7 @@ func TestSetDefaults(t *testing.T) { Resources: &MRuntimeResources, }, }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &one, - MaxReplicas: &one, - }, + Replicas: &one, }}, }, "Should properly merge resources presets (using labels) - case with missing buildResources Requests": { @@ -512,6 +492,7 @@ func TestSetDefaults(t *testing.T) { Function: ResourceRequirementsBuilder{}.Limits("50m", "64Mi").Requests("15m", "15Mi").Build(), Build: ResourceRequirementsBuilder{}.Limits("700m", "700Mi").Requests("350m", "350Mi").Build(), }, + Replicas: &two, ScaleConfig: &ScaleConfig{ MinReplicas: &two, MaxReplicas: &two, @@ -540,6 +521,7 @@ func TestSetDefaults(t *testing.T) { Function: ResourceRequirementsBuilder{}.Limits("50m", "64Mi").Requests("15m", "15Mi").Profile("S").Build(), Build: ResourceRequirementsBuilder{}.Limits("700m", "700Mi").Requests("350m", "350Mi").Profile("slow").Build(), }, + Replicas: &two, ScaleConfig: &ScaleConfig{ MinReplicas: &two, MaxReplicas: &two, @@ -568,10 +550,7 @@ func TestSetDefaults(t *testing.T) { ResourceConfiguration: &ResourceConfiguration{ Function: ResourceRequirementsBuilder{}.Limits("100m", "128Mi").Requests("50m", "64Mi").Build(), }, - ScaleConfig: &ScaleConfig{ - MinReplicas: &one, - MaxReplicas: &one, - }, + Replicas: &one, }, }, }, diff --git a/components/function-controller/pkg/apis/serverless/v1alpha2/function_validation.go b/components/function-controller/pkg/apis/serverless/v1alpha2/function_validation.go index 7eb92876e409..b02d182a37eb 100644 --- a/components/function-controller/pkg/apis/serverless/v1alpha2/function_validation.go +++ b/components/function-controller/pkg/apis/serverless/v1alpha2/function_validation.go @@ -297,9 +297,7 @@ func (spec *FunctionSpec) validateReplicas(vc *ValidationConfig) error { maxReplicas = spec.ScaleConfig.MaxReplicas minReplicas = spec.ScaleConfig.MinReplicas allErrs := []string{} - if spec.Replicas != nil && spec.ScaleConfig != nil { - allErrs = append(allErrs, "spec.replicas and spec.scaleConfig are use at the same time") - } + if spec.Replicas == nil && spec.ScaleConfig == nil { allErrs = append(allErrs, "spec.replicas and spec.scaleConfig are empty at the same time") } diff --git a/components/function-controller/pkg/apis/serverless/v1alpha2/function_validation_test.go b/components/function-controller/pkg/apis/serverless/v1alpha2/function_validation_test.go index 63fe67307880..dc1a869d1346 100644 --- a/components/function-controller/pkg/apis/serverless/v1alpha2/function_validation_test.go +++ b/components/function-controller/pkg/apis/serverless/v1alpha2/function_validation_test.go @@ -575,7 +575,7 @@ func TestFunctionSpec_validateResources(t *testing.T) { ), expectedError: gomega.HaveOccurred(), }, - "Should return error because replicas field is use together with scaleConfig": { + "Should not return error when replicas field is use together with scaleConfig": { givenFunc: Function{ ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "test"}, Spec: FunctionSpec{ @@ -617,7 +617,7 @@ func TestFunctionSpec_validateResources(t *testing.T) { }, }, }, - expectedError: gomega.HaveOccurred(), + expectedError: gomega.BeNil(), }, "Should validate without error Resources and Profile occurring at once in ResourceConfiguration.Function/Build": { givenFunc: Function{ diff --git a/resources/serverless/values.yaml b/resources/serverless/values.yaml index 7b2899300fec..82994a00c766 100644 --- a/resources/serverless/values.yaml +++ b/resources/serverless/values.yaml @@ -76,13 +76,13 @@ global: directory: "tpi" function_controller: name: "function-controller" - version: "PR-15761" + version: "PR-15576" function_webhook: name: "function-webhook" - version: "PR-15761" + version: "PR-15576" function_build_init: name: "function-build-init" - version: "PR-15761" + version: "PR-15576" function_runtime_nodejs12: name: "function-runtime-nodejs12" version: "PR-15671" @@ -110,7 +110,7 @@ global: testImages: function_controller_test: name: "function-controller-test" - version: "05ca702a" + version: "PR-15576" git_server: name: "gitserver" version: "708f6a87" diff --git a/tests/function-controller/go.mod b/tests/function-controller/go.mod index fe95fa7d065b..51357ceef66f 100644 --- a/tests/function-controller/go.mod +++ b/tests/function-controller/go.mod @@ -9,7 +9,7 @@ require ( github.com/hashicorp/errwrap v1.0.0 github.com/hashicorp/go-multierror v1.1.0 github.com/kyma-project/kyma/components/eventing-controller v0.0.0-20220915084356-1d9b39c6797a - github.com/kyma-project/kyma/components/function-controller v0.0.0-20210708083136-5479837a0948 + github.com/kyma-project/kyma/components/function-controller v0.0.0-20220928064850-1c007274729b github.com/onsi/gomega v1.20.2 github.com/pkg/errors v0.9.1 github.com/sirupsen/logrus v1.8.1 @@ -23,12 +23,13 @@ require ( replace ( github.com/googleapis/gnostic => github.com/googleapis/gnostic v0.3.1 + github.com/kyma-incubator/api-gateway => github.com/kyma-project/api-gateway v0.0.0-20220819093753-296e6704d413 //TODO: replace it in another PR - github.com/kyma-project/kyma/components/function-controller => github.com/pPrecel/kyma/components/function-controller v0.0.0-20220801083447-41d01d8dc0c7 go.etcd.io/etcd => go.etcd.io/etcd v3.3.25+incompatible golang.org/x/crypto => golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a golang.org/x/text => golang.org/x/text v0.3.3 k8s.io/kube-openapi => k8s.io/kube-openapi v0.0.0-20200410145947-bcb3869e6f29 + ) replace k8s.io/api => k8s.io/api v0.18.12 diff --git a/tests/function-controller/go.sum b/tests/function-controller/go.sum index 6b9b42b7f315..b9c9fb8bbbeb 100644 --- a/tests/function-controller/go.sum +++ b/tests/function-controller/go.sum @@ -427,13 +427,15 @@ github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/kyma-incubator/api-gateway v0.0.0-20220819093753-296e6704d413/go.mod h1:5kBV6C2JEaapjTAn10Mo81Te4e6LN3epexUCSLXgQLI= +github.com/kyma-project/api-gateway v0.0.0-20220819093753-296e6704d413/go.mod h1:5kBV6C2JEaapjTAn10Mo81Te4e6LN3epexUCSLXgQLI= github.com/kyma-project/kyma/common/logging v0.0.0-20220602092229-f2e29f34ed5e/go.mod h1:7FWH0Lyls2xumj836aa+LVP8jhnJSv6wSlxC+2HAJ1s= github.com/kyma-project/kyma/common/logging v0.0.0-20220903121145-690b76935712/go.mod h1:oXoP77o6Am2IWp8wDS3jaA1gGWLrcaO6gLWyDZbAkJs= github.com/kyma-project/kyma/components/application-operator v0.0.0-20220903121145-690b76935712/go.mod h1:4jh1Qn1DLbQRdCHqEdHsV4Tk9b6L4i9nhgGdEc2XaIg= github.com/kyma-project/kyma/components/eventing-controller v0.0.0-20220915084356-1d9b39c6797a h1:t1wNtM9GV8GBuPdjk6646iWPlf1nkQs6rGTLYAbDAxA= github.com/kyma-project/kyma/components/eventing-controller v0.0.0-20220915084356-1d9b39c6797a/go.mod h1:ATzbpEZqwuawkwCfI8jegw3ItXw+3s2WuKgEhbUI4Jw= -github.com/libgit2/git2go/v31 v31.4.14/go.mod h1:c/rkJcBcUFx6wHaT++UwNpKvIsmPNqCeQ/vzO4DrEec= +github.com/kyma-project/kyma/components/function-controller v0.0.0-20220928064850-1c007274729b h1:lBdnMyh4LIHgCv7IoAvr2h06oY6pf71O98XV2xlAIRw= +github.com/kyma-project/kyma/components/function-controller v0.0.0-20220928064850-1c007274729b/go.mod h1:yKPrqviLs4EX6fgpNUgt9n9Xg5PPEut4GcLeNlw7Us0= +github.com/libgit2/git2go/v31 v31.7.9/go.mod h1:c/rkJcBcUFx6wHaT++UwNpKvIsmPNqCeQ/vzO4DrEec= github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60= github.com/mailru/easyjson v0.0.0-20160728113105-d5b7844b561a/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= @@ -517,8 +519,6 @@ github.com/onsi/gomega v1.20.2 h1:8uQq0zMgLEfa0vRrrBgaJF2gyW9Da9BmfGV+OyUzfkY= github.com/onsi/gomega v1.20.2/go.mod h1:iYAIXgPSaDHak0LCMA+AWBpIKBr8WZicMxnE8luStNc= github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/ory/oathkeeper-maester v0.1.7/go.mod h1:RkZtgWAwgbkb8o5kHm0WlURRJr9ZjPpF8NQkkviThBo= -github.com/pPrecel/kyma/components/function-controller v0.0.0-20220801083447-41d01d8dc0c7 h1:fqIpH+E9OD6pR9meM6jdDqI5GW7P+w7JE34tDGWxOAc= -github.com/pPrecel/kyma/components/function-controller v0.0.0-20220801083447-41d01d8dc0c7/go.mod h1:pUzJCeMD6jhiUbjks+uS11zd6U0MFKD4v79nrKWCj3k= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/pelletier/go-toml v1.9.3/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCkoOuaOx1Y+c= diff --git a/tests/function-controller/testsuite/teststep/check.go b/tests/function-controller/testsuite/teststep/check.go index c04d0caccccc..23aaa21e1b80 100644 --- a/tests/function-controller/testsuite/teststep/check.go +++ b/tests/function-controller/testsuite/teststep/check.go @@ -101,12 +101,8 @@ func (d DefaultedFunctionCheck) Run() error { } spec := fn.Spec - if spec.ScaleConfig == nil { - return errors.New("scaleConfig equal nil") - } else if spec.ScaleConfig.MinReplicas == nil { - return errors.New("minReplicas equal nil") - } else if spec.ScaleConfig.MaxReplicas == nil { - return errors.New("maxReplicas equal nil") + if spec.Replicas == nil { + return errors.New("replicas equal nil") } else if spec.ResourceConfiguration.Function.Resources.Requests.Memory().IsZero() || spec.ResourceConfiguration.Function.Resources.Requests.Cpu().IsZero() { return errors.New("requests equal zero") } else if spec.ResourceConfiguration.Function.Resources.Limits.Memory().IsZero() || spec.ResourceConfiguration.Function.Resources.Limits.Cpu().IsZero() {