Skip to content

Commit 64ff5d2

Browse files
ntkatholentkathole
committed
fix: Server side apply
Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
1 parent 24afd55 commit 64ff5d2

File tree

2 files changed

+70
-48
lines changed

2 files changed

+70
-48
lines changed

‎infra/feast-operator/internal/controller/services/scaling.go‎

Lines changed: 50 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,14 @@ import (
2222
autoscalingv2 "k8s.io/api/autoscaling/v2"
2323
corev1 "k8s.io/api/core/v1"
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25-
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
25+
"sigs.k8s.io/controller-runtime/pkg/client"
2626
"sigs.k8s.io/controller-runtime/pkg/log"
2727
)
2828

2929
const (
3030
defaultHPACPUUtilization int32 = 80
3131
defaultHPAMinReplicas int32 = 1
32+
fieldManager = "feast-operator"
3233
)
3334

3435
// getDesiredReplicas returns the replica count the operator should set on the
@@ -49,73 +50,82 @@ func (feast *FeastServices) getDesiredReplicas() *int32 {
4950
}
5051

5152
// createOrDeleteHPA reconciles the HorizontalPodAutoscaler for the FeatureStore
52-
// deployment. If autoscaling is not configured, any existing HPA is deleted.
53+
// deployment using Server-Side Apply. If autoscaling is not configured, any
54+
// existing HPA is deleted.
5355
func (feast *FeastServices) createOrDeleteHPA() error {
5456
cr := feast.Handler.FeatureStore
55-
hpa := feast.initHPA()
5657

5758
scaling := cr.Status.Applied.Services.Scaling
5859
if scaling == nil || scaling.Autoscaling == nil {
60+
hpa := &autoscalingv2.HorizontalPodAutoscaler{
61+
ObjectMeta: feast.GetObjectMeta(),
62+
}
63+
hpa.SetGroupVersionKind(autoscalingv2.SchemeGroupVersion.WithKind("HorizontalPodAutoscaler"))
5964
return feast.Handler.DeleteOwnedFeastObj(hpa)
6065
}
6166

67+
hpa := feast.buildHPA()
6268
logger := log.FromContext(feast.Handler.Context)
63-
if op, err := controllerutil.CreateOrUpdate(feast.Handler.Context, feast.Handler.Client, hpa, controllerutil.MutateFn(func() error {
64-
return feast.setHPA(hpa)
65-
})); err != nil {
69+
if err := feast.Handler.Client.Patch(feast.Handler.Context, hpa,
70+
client.Apply, client.FieldOwner(fieldManager), client.ForceOwnership); err != nil {
6671
return err
67-
} else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated {
68-
logger.Info("Successfully reconciled", "HorizontalPodAutoscaler", hpa.Name, "operation", op)
6972
}
73+
logger.Info("Successfully applied", "HorizontalPodAutoscaler", hpa.Name)
7074

7175
return nil
7276
}
7377

74-
func (feast *FeastServices) initHPA() *autoscalingv2.HorizontalPodAutoscaler {
75-
hpa := &autoscalingv2.HorizontalPodAutoscaler{
76-
ObjectMeta: feast.GetObjectMeta(),
77-
}
78-
hpa.SetGroupVersionKind(autoscalingv2.SchemeGroupVersion.WithKind("HorizontalPodAutoscaler"))
79-
return hpa
80-
}
81-
82-
func (feast *FeastServices) setHPA(hpa *autoscalingv2.HorizontalPodAutoscaler) error {
78+
// buildHPA constructs the fully desired HPA state for Server-Side Apply.
79+
func (feast *FeastServices) buildHPA() *autoscalingv2.HorizontalPodAutoscaler {
8380
cr := feast.Handler.FeatureStore
84-
scaling := cr.Status.Applied.Services.Scaling
85-
if scaling == nil || scaling.Autoscaling == nil {
86-
return nil
87-
}
88-
autoscaling := scaling.Autoscaling
89-
90-
hpa.Labels = feast.getLabels()
81+
autoscaling := cr.Status.Applied.Services.Scaling.Autoscaling
9182

9283
deploy := feast.initFeastDeploy()
9384
minReplicas := defaultHPAMinReplicas
9485
if autoscaling.MinReplicas != nil {
9586
minReplicas = *autoscaling.MinReplicas
9687
}
9788

98-
hpa.Spec = autoscalingv2.HorizontalPodAutoscalerSpec{
99-
ScaleTargetRef: autoscalingv2.CrossVersionObjectReference{
100-
APIVersion: "apps/v1",
101-
Kind: "Deployment",
102-
Name: deploy.Name,
103-
},
104-
MinReplicas: &minReplicas,
105-
MaxReplicas: autoscaling.MaxReplicas,
106-
}
107-
89+
metrics := defaultHPAMetrics()
10890
if len(autoscaling.Metrics) > 0 {
109-
hpa.Spec.Metrics = autoscaling.Metrics
110-
} else {
111-
hpa.Spec.Metrics = defaultHPAMetrics()
91+
metrics = autoscaling.Metrics
11292
}
11393

114-
if autoscaling.Behavior != nil {
115-
hpa.Spec.Behavior = autoscaling.Behavior
94+
isController := true
95+
hpa := &autoscalingv2.HorizontalPodAutoscaler{
96+
TypeMeta: metav1.TypeMeta{
97+
APIVersion: autoscalingv2.SchemeGroupVersion.String(),
98+
Kind: "HorizontalPodAutoscaler",
99+
},
100+
ObjectMeta: metav1.ObjectMeta{
101+
Name: feast.GetObjectMeta().Name,
102+
Namespace: feast.GetObjectMeta().Namespace,
103+
Labels: feast.getLabels(),
104+
OwnerReferences: []metav1.OwnerReference{
105+
{
106+
APIVersion: feastdevv1.GroupVersion.String(),
107+
Kind: "FeatureStore",
108+
Name: cr.Name,
109+
UID: cr.UID,
110+
Controller: &isController,
111+
BlockOwnerDeletion: &isController,
112+
},
113+
},
114+
},
115+
Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
116+
ScaleTargetRef: autoscalingv2.CrossVersionObjectReference{
117+
APIVersion: appsv1.SchemeGroupVersion.String(),
118+
Kind: "Deployment",
119+
Name: deploy.Name,
120+
},
121+
MinReplicas: &minReplicas,
122+
MaxReplicas: autoscaling.MaxReplicas,
123+
Metrics: metrics,
124+
Behavior: autoscaling.Behavior,
125+
},
116126
}
117127

118-
return controllerutil.SetControllerReference(cr, hpa, feast.Handler.Scheme)
128+
return hpa
119129
}
120130

121131
func defaultHPAMetrics() []autoscalingv2.MetricSpec {

‎infra/feast-operator/internal/controller/services/scaling_test.go‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -550,31 +550,29 @@ var _ = Describe("Horizontal Scaling", func() {
550550
})
551551

552552
Describe("HPA Configuration", func() {
553-
It("should create an HPA with default CPU metrics", func() {
553+
It("should build an HPA with default CPU metrics", func() {
554554
featureStore.Status.Applied.Services.Scaling = &feastdevv1.ScalingConfig{
555555
Autoscaling: &feastdevv1.AutoscalingConfig{
556556
MaxReplicas: 10,
557557
},
558558
}
559559

560-
hpa := feast.initHPA()
561-
Expect(feast.setHPA(hpa)).To(Succeed())
560+
hpa := feast.buildHPA()
562561
Expect(hpa.Spec.MaxReplicas).To(Equal(int32(10)))
563562
Expect(*hpa.Spec.MinReplicas).To(Equal(int32(1)))
564563
Expect(hpa.Spec.Metrics).To(HaveLen(1))
565564
Expect(hpa.Spec.Metrics[0].Resource.Name).To(Equal(corev1.ResourceCPU))
566565
})
567566

568-
It("should create an HPA with custom min replicas", func() {
567+
It("should build an HPA with custom min replicas", func() {
569568
featureStore.Status.Applied.Services.Scaling = &feastdevv1.ScalingConfig{
570569
Autoscaling: &feastdevv1.AutoscalingConfig{
571570
MinReplicas: ptr(int32(2)),
572571
MaxReplicas: 10,
573572
},
574573
}
575574

576-
hpa := feast.initHPA()
577-
Expect(feast.setHPA(hpa)).To(Succeed())
575+
hpa := feast.buildHPA()
578576
Expect(*hpa.Spec.MinReplicas).To(Equal(int32(2)))
579577
Expect(hpa.Spec.MaxReplicas).To(Equal(int32(10)))
580578
})
@@ -586,12 +584,26 @@ var _ = Describe("Horizontal Scaling", func() {
586584
},
587585
}
588586

589-
hpa := feast.initHPA()
590-
Expect(feast.setHPA(hpa)).To(Succeed())
587+
hpa := feast.buildHPA()
591588
Expect(hpa.Spec.ScaleTargetRef.APIVersion).To(Equal("apps/v1"))
592589
Expect(hpa.Spec.ScaleTargetRef.Kind).To(Equal("Deployment"))
593590
Expect(hpa.Spec.ScaleTargetRef.Name).To(Equal(GetFeastName(featureStore)))
594591
})
592+
593+
It("should set TypeMeta and owner reference for SSA", func() {
594+
featureStore.Status.Applied.Services.Scaling = &feastdevv1.ScalingConfig{
595+
Autoscaling: &feastdevv1.AutoscalingConfig{
596+
MaxReplicas: 5,
597+
},
598+
}
599+
600+
hpa := feast.buildHPA()
601+
Expect(hpa.TypeMeta.APIVersion).To(Equal("autoscaling/v2"))
602+
Expect(hpa.TypeMeta.Kind).To(Equal("HorizontalPodAutoscaler"))
603+
Expect(hpa.OwnerReferences).To(HaveLen(1))
604+
Expect(hpa.OwnerReferences[0].Name).To(Equal(featureStore.Name))
605+
Expect(*hpa.OwnerReferences[0].Controller).To(BeTrue())
606+
})
595607
})
596608

597609
Describe("Scale sub-resource", func() {

0 commit comments

Comments
 (0)