-
Notifications
You must be signed in to change notification settings - Fork 571
feat: add extraContainers to SharedDeploymentSpec for sidecar support #1724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0991ec7
4c5f0c1
0a03435
a71c636
05a9c19
c4c9703
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ type resolvedDeployment struct { | |
| PodSecurityContext *corev1.PodSecurityContext | ||
| ServiceAccountName *string | ||
| ServiceAccountConfig *v1alpha2.ServiceAccountConfig | ||
| ExtraContainers []corev1.Container | ||
| } | ||
|
|
||
| // getDefaultResources sets default resource requirements if not specified | ||
|
|
@@ -105,6 +106,22 @@ func getRuntimeImageRepository(runtime v1alpha2.DeclarativeRuntime) string { | |
| } | ||
| } | ||
|
|
||
| // validateExtraContainers checks that none of the extra containers use the | ||
| // reserved name "kagent" and that no two containers share the same name. | ||
| func validateExtraContainers(containers []corev1.Container) error { | ||
| seen := make(map[string]bool) | ||
| for _, c := range containers { | ||
| if c.Name == "kagent" { | ||
| return fmt.Errorf("extraContainers: %q is a reserved container name", c.Name) | ||
| } | ||
| if seen[c.Name] { | ||
| return fmt.Errorf("extraContainers: duplicate container name %q", c.Name) | ||
| } | ||
| seen[c.Name] = true | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func resolveInlineDeployment(agent v1alpha2.AgentObject, mdd *modelDeploymentData) (*resolvedDeployment, error) { | ||
| specRef := agent.GetAgentSpec() | ||
| // Defaults | ||
|
|
@@ -161,6 +178,10 @@ func resolveInlineDeployment(agent v1alpha2.AgentObject, mdd *modelDeploymentDat | |
| } | ||
| } | ||
|
|
||
| if err := validateExtraContainers(spec.ExtraContainers); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| dep := &resolvedDeployment{ | ||
| Image: image, | ||
| Args: args, | ||
|
|
@@ -181,6 +202,7 @@ func resolveInlineDeployment(agent v1alpha2.AgentObject, mdd *modelDeploymentDat | |
| PodSecurityContext: spec.PodSecurityContext, | ||
| ServiceAccountName: spec.ServiceAccountName, | ||
| ServiceAccountConfig: spec.ServiceAccountConfig, | ||
| ExtraContainers: slices.Clone(spec.ExtraContainers), | ||
|
||
| } | ||
|
|
||
| // Precedence: agent-level serviceAccountName > global default > auto-created SA (agent name) | ||
|
|
@@ -241,6 +263,10 @@ func resolveByoDeployment(agent v1alpha2.AgentObject) (*resolvedDeployment, erro | |
| replicas = new(int32(1)) | ||
| } | ||
|
|
||
| if err := validateExtraContainers(spec.ExtraContainers); err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| dep := &resolvedDeployment{ | ||
| Image: image, | ||
| Cmd: cmd, | ||
|
|
@@ -262,6 +288,7 @@ func resolveByoDeployment(agent v1alpha2.AgentObject) (*resolvedDeployment, erro | |
| PodSecurityContext: spec.PodSecurityContext, | ||
| ServiceAccountName: spec.ServiceAccountName, | ||
| ServiceAccountConfig: spec.ServiceAccountConfig, | ||
| ExtraContainers: slices.Clone(spec.ExtraContainers), | ||
| } | ||
|
|
||
| // Precedence: agent-level serviceAccountName > global default > auto-created SA (agent name) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package agent | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| corev1 "k8s.io/api/core/v1" | ||
| ) | ||
|
|
||
| func TestValidateExtraContainers(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| containers []corev1.Container | ||
| wantErr bool | ||
| }{ | ||
| { | ||
| name: "empty list is fine", | ||
| containers: nil, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "normal sidecar names are fine", | ||
| containers: []corev1.Container{ | ||
| {Name: "envoy"}, | ||
| {Name: "log-shipper"}, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "reserved name kagent is rejected", | ||
| containers: []corev1.Container{ | ||
| {Name: "kagent"}, | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| { | ||
| name: "duplicate sidecar names are rejected", | ||
| containers: []corev1.Container{ | ||
| {Name: "proxy"}, | ||
| {Name: "proxy"}, | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| { | ||
| name: "kagent mixed with other containers is still rejected", | ||
| containers: []corev1.Container{ | ||
| {Name: "envoy"}, | ||
| {Name: "kagent"}, | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
| err := validateExtraContainers(tt.containers) | ||
| if (err != nil) != tt.wantErr { | ||
| t.Errorf("validateExtraContainers() error = %v, wantErr %v", err, tt.wantErr) | ||
| } | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| operation: translateAgent | ||
| targetObject: agent-with-extra-containers | ||
| namespace: test | ||
| objects: | ||
| - apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: openai-secret | ||
| namespace: test | ||
| data: | ||
| api-key: c2stdGVzdC1hcGkta2V5 # base64 encoded "sk-test-api-key" | ||
| - apiVersion: kagent.dev/v1alpha2 | ||
| kind: ModelConfig | ||
| metadata: | ||
| name: basic-model | ||
| namespace: test | ||
| spec: | ||
| provider: OpenAI | ||
| model: gpt-4o | ||
| apiKeySecret: openai-secret | ||
| apiKeySecretKey: api-key | ||
| - apiVersion: kagent.dev/v1alpha2 | ||
| kind: Agent | ||
| metadata: | ||
| name: agent-with-extra-containers | ||
| namespace: test | ||
| spec: | ||
| type: Declarative | ||
| declarative: | ||
| description: A test agent with a sidecar container | ||
| systemMessage: You are a helpful assistant. | ||
| modelConfig: basic-model | ||
| deployment: | ||
| extraContainers: | ||
| - name: token-proxy | ||
| image: envoyproxy/envoy:v1.30.0 | ||
| args: | ||
| - -c | ||
| - /etc/envoy/envoy.yaml | ||
| ports: | ||
| - containerPort: 9901 | ||
| name: admin | ||
| resources: | ||
| requests: | ||
| cpu: 50m | ||
| memory: 64Mi | ||
| limits: | ||
| cpu: 200m | ||
| memory: 128Mi | ||
| tools: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exposing
[]corev1.Containerdirectly in a CRD can significantly bloat the generated OpenAPI schema and can also prune unknown/newer container fields (apiserver schema-pruning) as Kubernetes evolves. To avoid CRD size/compatibility issues, consider marking this field as schemaless / preserve-unknown-fields (kubebuilder marker) or switching to a slimmer custom sidecar spec / raw JSON representation, depending on how strictly you want to validate user input.