Skip to content
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

Set k8s default container label #15694

Merged
merged 9 commits into from
Jan 16, 2025
3 changes: 2 additions & 1 deletion pkg/reconciler/revision/resources/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ func MakeDeployment(rev *v1.Revision, cfg *config.Config) (*appsv1.Deployment, e

labels := makeLabels(rev)
anns := makeAnnotations(rev)
annsPod := makeAnnotationsForPod(rev, anns)

// Slowly but steadily roll the deployment out, to have the least possible impact.
maxUnavailable := intstr.FromInt(0)
Expand All @@ -399,7 +400,7 @@ func MakeDeployment(rev *v1.Revision, cfg *config.Config) (*appsv1.Deployment, e
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: labels,
Annotations: anns,
Annotations: annsPod,
},
Spec: *podSpec,
},
Expand Down
31 changes: 24 additions & 7 deletions pkg/reconciler/revision/resources/deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,9 @@ var (
serving.RevisionUID: "1234",
AppLabelKey: "bar",
},
Annotations: map[string]string{},
Annotations: map[string]string{
DefaultContainerAnnotationName: servingContainerName,
},
},
// Spec: filled in by makePodSpec
},
Expand Down Expand Up @@ -1829,8 +1831,13 @@ func TestMakeDeployment(t *testing.T) {
}}), withoutLabels),
want: appsv1deployment(func(deploy *appsv1.Deployment) {
deploy.Spec.ProgressDeadlineSeconds = ptr.Int32(42)
deploy.Annotations = map[string]string{serving.ProgressDeadlineAnnotationKey: "42s"}
deploy.Spec.Template.Annotations = map[string]string{serving.ProgressDeadlineAnnotationKey: "42s"}
deploy.Annotations = map[string]string{
serving.ProgressDeadlineAnnotationKey: "42s",
}
deploy.Spec.Template.Annotations = map[string]string{
DefaultContainerAnnotationName: servingContainerName,
serving.ProgressDeadlineAnnotationKey: "42s",
}
}),
}, {
name: "with ProgressDeadline annotation and configmap override",
Expand All @@ -1849,8 +1856,13 @@ func TestMakeDeployment(t *testing.T) {
}}), withoutLabels),
want: appsv1deployment(func(deploy *appsv1.Deployment) {
deploy.Spec.ProgressDeadlineSeconds = ptr.Int32(42)
deploy.Annotations = map[string]string{serving.ProgressDeadlineAnnotationKey: "42s"}
deploy.Spec.Template.Annotations = map[string]string{serving.ProgressDeadlineAnnotationKey: "42s"}
deploy.Annotations = map[string]string{
serving.ProgressDeadlineAnnotationKey: "42s",
}
deploy.Spec.Template.Annotations = map[string]string{
DefaultContainerAnnotationName: servingContainerName,
serving.ProgressDeadlineAnnotationKey: "42s",
}
}),
}, {
name: "cluster initial scale",
Expand Down Expand Up @@ -1886,8 +1898,13 @@ func TestMakeDeployment(t *testing.T) {
),
want: appsv1deployment(func(deploy *appsv1.Deployment) {
deploy.Spec.Replicas = ptr.Int32(int32(20))
deploy.Spec.Template.Annotations = map[string]string{autoscaling.InitialScaleAnnotationKey: "20"}
deploy.Annotations = map[string]string{autoscaling.InitialScaleAnnotationKey: "20"}
deploy.Annotations = map[string]string{
autoscaling.InitialScaleAnnotationKey: "20",
}
deploy.Spec.Template.Annotations = map[string]string{
autoscaling.InitialScaleAnnotationKey: "20",
DefaultContainerAnnotationName: servingContainerName,
}
}),
}}

Expand Down
17 changes: 17 additions & 0 deletions pkg/reconciler/revision/resources/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package resources

import (
"maps"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/sets"
"knative.dev/pkg/kmeta"
Expand All @@ -38,6 +40,10 @@ var (
)
)

const (
DefaultContainerAnnotationName = "kubectl.kubernetes.io/default-container"
)

// makeLabels constructs the labels we will apply to K8s resources.
func makeLabels(revision *v1.Revision) map[string]string {
labels := kmeta.FilterMap(revision.GetLabels(), excludeLabels.Has)
Expand All @@ -59,6 +65,17 @@ func makeAnnotations(revision *v1.Revision) map[string]string {
return kmeta.FilterMap(revision.GetAnnotations(), excludeAnnotations.Has)
}

func makeAnnotationsForPod(revision *v1.Revision, baseAnnotations map[string]string) map[string]string {
podAnnotations := maps.Clone(baseAnnotations)

// Add default container annotation to the pod meta
if userContainer := revision.Spec.GetContainer(); userContainer.Name != "" {
podAnnotations[DefaultContainerAnnotationName] = userContainer.Name
}

return podAnnotations
}

// makeSelector constructs the Selector we will apply to K8s resources.
func makeSelector(revision *v1.Revision) *metav1.LabelSelector {
return &metav1.LabelSelector{
Expand Down
49 changes: 49 additions & 0 deletions pkg/reconciler/revision/resources/meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/google/go-cmp/cmp"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"knative.dev/serving/pkg/apis/serving"
Expand Down Expand Up @@ -141,3 +142,51 @@ func TestMakeAnnotations(t *testing.T) {
})
}
}

func TestMakeAnnotationsForPod(t *testing.T) {
tests := []struct {
name string
rev *v1.Revision
want map[string]string
baseAnnotations map[string]string
}{{
name: "no container",
rev: &v1.Revision{
Spec: v1.RevisionSpec{
PodSpec: corev1.PodSpec{},
},
},
baseAnnotations: map[string]string{},
want: map[string]string{},
}, {
konstfish marked this conversation as resolved.
Show resolved Hide resolved
name: "multiple containers single port with base annotation",
rev: &v1.Revision{
Spec: v1.RevisionSpec{
PodSpec: corev1.PodSpec{
Containers: []corev1.Container{{
Name: "foo",
}, {
Name: "bar",
Ports: []corev1.ContainerPort{{
ContainerPort: 8080,
}},
}},
},
},
},
baseAnnotations: map[string]string{"asdf": "fdsa"},
want: map[string]string{
"asdf": "fdsa",
DefaultContainerAnnotationName: "bar",
},
}}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got := makeAnnotationsForPod(test.rev, test.baseAnnotations)
if diff := cmp.Diff(test.want, got); diff != "" {
t.Error("getUserContainerName (-want, +got) =", diff)
}
})
}
}