Commit 7887ffd3 authored by julienmancuso's avatar julienmancuso Committed by GitHub
Browse files

feat: allow replicas to be set in DynamoDeployment CR (#486)

parent 77c28919
......@@ -39,7 +39,7 @@ cd ../..
cd components/api-server
helm dependency update
cd ../..
helm dep build
helm dep update
helm repo update
cd ..
......
......@@ -23,7 +23,7 @@ version: 25.2.0-rc3
home: https://nvidia.com
dependencies:
- name: dynamo-operator
version: 0.1.0
version: 0.1.1
repository: file://components/operator
condition: dynamo-operator.enabled
- name: dynamo-api-server
......
......@@ -27,7 +27,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 0.1.0
version: 0.1.1
# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
# follow Semantic Versioning. They should reflect the version the application is using.
......@@ -35,5 +35,5 @@ version: 0.1.0
appVersion: "0.1.0"
dependencies:
- name: dynamo-crds
version: 0.1.0
version: 0.1.1
repository: file://charts/dynamo-crds
\ No newline at end of file
......@@ -16,5 +16,5 @@ apiVersion: v2
name: dynamo-crds
description: A Helm chart for CRDs of dynamo operator
type: application
version: 0.1.0
version: 0.1.1
dependencies: []
\ No newline at end of file
......@@ -392,6 +392,8 @@ spec:
type: object
dynamoNim:
type: string
dynamoTag:
type: string
envFromSecret:
type: string
envs:
......@@ -2747,6 +2749,9 @@ spec:
format: int32
type: integer
type: object
replicas:
format: int32
type: integer
resources:
properties:
limits:
......@@ -2783,8 +2788,6 @@ spec:
type: object
serviceName:
type: string
required:
- dynamoNim
type: object
status:
properties:
......
......@@ -2748,6 +2748,9 @@ spec:
format: int32
type: integer
type: object
replicas:
format: int32
type: integer
resources:
properties:
limits:
......
//go:build !ignore_autogenerated
/*
* SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*
* 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.
*/
SPDX-FileCopyrightText: Copyright (c) 2024-2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
SPDX-License-Identifier: Apache-2.0
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.
*/
/*
Copyright 2024.
......
......@@ -70,3 +70,11 @@ type DynamoDeploymentList struct {
func init() {
SchemeBuilder.Register(&DynamoDeployment{}, &DynamoDeploymentList{})
}
func (s *DynamoDeployment) GetSpec() any {
return s.Spec
}
func (s *DynamoDeployment) SetSpec(spec any) {
s.Spec = spec.(DynamoDeploymentSpec)
}
......@@ -66,6 +66,7 @@ type DynamoNimDeploymentSpec struct {
LivenessProbe *corev1.Probe `json:"livenessProbe,omitempty"`
ReadinessProbe *corev1.Probe `json:"readinessProbe,omitempty"`
Replicas *int32 `json:"replicas,omitempty"`
}
type RunMode struct {
......@@ -137,3 +138,11 @@ func (s *DynamoNimDeploymentStatus) IsReady() bool {
}
return false
}
func (s *DynamoNimDeployment) GetSpec() any {
return s.Spec
}
func (s *DynamoNimDeployment) SetSpec(spec any) {
s.Spec = spec.(DynamoNimDeploymentSpec)
}
......@@ -113,3 +113,11 @@ type DynamoNimRequestList struct {
func init() {
SchemeBuilder.Register(&DynamoNimRequest{}, &DynamoNimRequestList{})
}
func (s *DynamoNimRequest) GetSpec() any {
return s.Spec
}
func (s *DynamoNimRequest) SetSpec(spec any) {
s.Spec = spec.(DynamoNimRequestSpec)
}
......@@ -430,6 +430,11 @@ func (in *DynamoNimDeploymentSpec) DeepCopyInto(out *DynamoNimDeploymentSpec) {
*out = new(corev1.Probe)
(*in).DeepCopyInto(*out)
}
if in.Replicas != nil {
in, out := &in.Replicas, &out.Replicas
*out = new(int32)
**out = **in
}
}
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DynamoNimDeploymentSpec.
......
......@@ -2749,6 +2749,9 @@ spec:
format: int32
type: integer
type: object
replicas:
format: int32
type: integer
resources:
properties:
limits:
......@@ -2785,9 +2788,6 @@ spec:
type: object
serviceName:
type: string
required:
- dynamoNim
- dynamoTag
type: object
status:
properties:
......
......@@ -2748,6 +2748,9 @@ spec:
format: int32
type: integer
type: object
replicas:
format: int32
type: integer
resources:
properties:
limits:
......
......@@ -145,7 +145,7 @@ func (r *DynamoDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
reason = "failed_to_set_the_controller_reference_for_the_DynamoNimRequest"
return ctrl.Result{}, err
}
_, err = commonController.SyncResource(ctx, r.Client, dynamoNimRequest, types.NamespacedName{Name: dynamoNimRequest.Name, Namespace: dynamoNimRequest.Namespace}, true)
_, err = commonController.SyncResource(ctx, r.Client, dynamoNimRequest, types.NamespacedName{Name: dynamoNimRequest.Name, Namespace: dynamoNimRequest.Namespace}, false)
if err != nil {
reason = "failed_to_sync_the_DynamoNimRequest"
return ctrl.Result{}, err
......@@ -159,7 +159,7 @@ func (r *DynamoDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
reason = "failed_to_set_the_controller_reference_for_the_DynamoNimDeployment"
return ctrl.Result{}, err
}
dynamoNimDeployment, err = commonController.SyncResource(ctx, r.Client, dynamoNimDeployment, types.NamespacedName{Name: dynamoNimDeployment.Name, Namespace: dynamoNimDeployment.Namespace}, true)
dynamoNimDeployment, err = commonController.SyncResource(ctx, r.Client, dynamoNimDeployment, types.NamespacedName{Name: dynamoNimDeployment.Name, Namespace: dynamoNimDeployment.Namespace}, false)
if err != nil {
reason = "failed_to_sync_the_DynamoNimDeployment"
return ctrl.Result{}, err
......
......@@ -1483,6 +1483,7 @@ func (r *DynamoNimDeploymentReconciler) generateDeployment(ctx context.Context,
}
var replicas *int32
replicas = opt.dynamoNimDeployment.Spec.Replicas
if opt.isStealingTrafficDebugModeEnabled {
replicas = &[]int32{int32(1)}[0]
}
......
......@@ -35,7 +35,13 @@ const (
NvidiaAnnotationHashKey = "nvidia.com/last-applied-hash"
)
func SyncResource[T client.Object](ctx context.Context, c client.Client, desired T, namespacedName types.NamespacedName, createOnly bool) (T, error) {
type Resource interface {
client.Object
GetSpec() any
SetSpec(spec any)
}
func SyncResource[T Resource](ctx context.Context, c client.Client, desired T, namespacedName types.NamespacedName, createOnly bool) (T, error) {
// Retrieve the GroupVersionKind (GVK) of the desired object
gvk, err := apiutil.GVKForObject(desired, c.Scheme())
if err != nil {
......@@ -73,7 +79,9 @@ func SyncResource[T client.Object](ctx context.Context, c client.Client, desired
// Check if the Spec has changed and update if necessary
if IsSpecChanged(current, desired) {
if err := c.Update(ctx, desired); err != nil {
// update the spec of the current object with the desired spec
current.SetSpec(desired.GetSpec())
if err := c.Update(ctx, current); err != nil {
return desired, fmt.Errorf("failed to update resource: %w", err)
}
}
......@@ -83,7 +91,7 @@ func SyncResource[T client.Object](ctx context.Context, c client.Client, desired
}
// GetResourceHash returns a consistent hash for the given object spec
func GetResourceHash(obj client.Object) string {
func GetResourceHash(obj any) string {
// Convert obj to a map[string]interface{}
objMap, err := json.Marshal(obj)
if err != nil {
......@@ -112,12 +120,12 @@ func GetResourceHash(obj client.Object) string {
// IsSpecChanged returns true if the spec has changed between the existing one
// and the new resource spec compared by hash.
func IsSpecChanged(current client.Object, desired client.Object) bool {
func IsSpecChanged(current Resource, desired Resource) bool {
if current == nil && desired != nil {
return true
}
hashStr := GetResourceHash(desired)
hashStr := GetResourceHash(desired.GetSpec())
foundHashAnnotation := false
currentAnnotations := current.GetAnnotations()
......
......@@ -21,52 +21,65 @@ import (
"testing"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"sigs.k8s.io/controller-runtime/pkg/client"
)
type MyResource struct {
unstructured.Unstructured
}
func (r *MyResource) GetSpec() any {
return r.Object["spec"]
}
func (r *MyResource) SetSpec(spec any) {
r.Object["spec"] = spec
}
func TestIsSpecChanged(t *testing.T) {
tests := []struct {
name string
current client.Object
desired client.Object
current Resource
desired Resource
expected bool
}{
{
name: "no change in hash with deployment spec and env variables",
current: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
current: &MyResource{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "value1"},
map[string]interface{}{"name": "ENV_VAR2", "value": "value2"},
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "value1"},
map[string]interface{}{"name": "ENV_VAR2", "value": "value2"},
},
},
},
......@@ -75,40 +88,42 @@ func TestIsSpecChanged(t *testing.T) {
},
},
},
desired: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
desired: &MyResource{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "value1"},
map[string]interface{}{"name": "ENV_VAR2", "value": "value2"},
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "value1"},
map[string]interface{}{"name": "ENV_VAR2", "value": "value2"},
},
},
},
......@@ -121,40 +136,42 @@ func TestIsSpecChanged(t *testing.T) {
},
{
name: "no change in hash with change in order of elements",
current: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
current: &MyResource{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
},
}, // switch order of env
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR2", "value": "value2"},
map[string]interface{}{"name": "ENV_VAR1", "value": "value1"},
},
}, // switch order of env
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR2", "value": "value2"},
map[string]interface{}{"name": "ENV_VAR1", "value": "value1"},
},
},
},
......@@ -163,40 +180,42 @@ func TestIsSpecChanged(t *testing.T) {
},
},
},
desired: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
desired: &MyResource{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "value1"},
map[string]interface{}{"name": "ENV_VAR2", "value": "value2"},
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "value1"},
map[string]interface{}{"name": "ENV_VAR2", "value": "value2"},
},
},
},
......@@ -209,40 +228,42 @@ func TestIsSpecChanged(t *testing.T) {
},
{
name: "change in hash with change in value of elements",
current: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
current: &MyResource{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"spec": map[string]interface{}{
"replicas": 2,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "value2"},
map[string]interface{}{"name": "ENV_VAR2", "value": "value1"},
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "value2"},
map[string]interface{}{"name": "ENV_VAR2", "value": "value1"},
},
},
},
......@@ -251,40 +272,42 @@ func TestIsSpecChanged(t *testing.T) {
},
},
},
desired: &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"spec": map[string]interface{}{
"replicas": 3,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
desired: &MyResource{
Unstructured: unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "apps/v1",
"kind": "Deployment",
"metadata": map[string]interface{}{
"name": "nim-deployment",
"namespace": "default",
},
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"spec": map[string]interface{}{
"replicas": 3,
"selector": map[string]interface{}{
"matchLabels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
"template": map[string]interface{}{
"metadata": map[string]interface{}{
"labels": map[string]interface{}{
"app": "nim",
},
},
"spec": map[string]interface{}{
"containers": []interface{}{
map[string]interface{}{
"name": "nim",
"image": "nim:v0.1.0",
"ports": []interface{}{
map[string]interface{}{
"containerPort": 80,
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "asdf"},
map[string]interface{}{"name": "ENV_VAR2", "value": "jljl"},
},
},
"env": []interface{}{
map[string]interface{}{"name": "ENV_VAR1", "value": "asdf"},
map[string]interface{}{"name": "ENV_VAR2", "value": "jljl"},
},
},
},
......@@ -300,10 +323,10 @@ func TestIsSpecChanged(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.current.SetAnnotations(map[string]string{
NvidiaAnnotationHashKey: GetResourceHash(tt.current),
NvidiaAnnotationHashKey: GetResourceHash(tt.current.GetSpec()),
})
if got := IsSpecChanged(tt.current, tt.desired); got != tt.expected {
t.Errorf("IsSpecChanged() = %v, want %v, hash current %s vs desired %s", got, tt.expected, GetResourceHash(tt.current), GetResourceHash(tt.desired))
t.Errorf("IsSpecChanged() = %v, want %v", got, tt.expected)
}
})
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment