Skip to content

Commit 1376cb1

Browse files
committed
Preserve deployment and template metadata during reconcile
When running `kubectl rollout restart deployment`, Kubernetes adds the `kubectl.kubernetes.io/restartedAt` annotation to the deployment's pod template to trigger a rolling restart. However, Knative's reconciler was removing this annotation by completely replacing the deployment spec, causing new pods to be immediately terminated instead of completing the rollout. This change preserves externally-added metadata at both the deployment level and template level during reconciliation by filtering Knative-controlled keys (*.knative.dev and app label) from the current state before merging with the desired state. This ensures Knative maintains full control over its own metadata while preserving external additions.
1 parent 60b1c39 commit 1376cb1

File tree

3 files changed

+123
-3
lines changed

3 files changed

+123
-3
lines changed

pkg/reconciler/revision/cruds.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@ package revision
1919
import (
2020
"context"
2121
"fmt"
22+
"strings"
2223

2324
appsv1 "k8s.io/api/apps/v1"
2425
"k8s.io/apimachinery/pkg/api/equality"
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627

2728
caching "knative.dev/caching/pkg/apis/caching/v1alpha1"
28-
"knative.dev/pkg/kmeta"
29+
"knative.dev/pkg/kmap"
2930
"knative.dev/pkg/kmp"
3031
"knative.dev/pkg/logging"
3132
autoscalingv1alpha1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1"
@@ -71,8 +72,13 @@ func (c *Reconciler) checkAndUpdateDeployment(ctx context.Context, rev *v1.Revis
7172
desiredDeployment := have.DeepCopy()
7273
desiredDeployment.Spec = deployment.Spec
7374

74-
// Carry over new labels.
75-
desiredDeployment.Labels = kmeta.UnionMaps(deployment.Labels, desiredDeployment.Labels)
75+
// Carry over new labels and annotations at deployment level.
76+
desiredDeployment.Labels = mergeMetadata(deployment.Labels, have.Labels)
77+
desiredDeployment.Annotations = mergeMetadata(deployment.Annotations, have.Annotations)
78+
79+
// Carry over template-level labels and annotations (e.g., kubectl.kubernetes.io/restartedAt from rollout restart).
80+
desiredDeployment.Spec.Template.Labels = mergeMetadata(deployment.Spec.Template.Labels, have.Spec.Template.Labels)
81+
desiredDeployment.Spec.Template.Annotations = mergeMetadata(deployment.Spec.Template.Annotations, have.Spec.Template.Annotations)
7682

7783
d, err := c.kubeclient.AppsV1().Deployments(deployment.Namespace).Update(ctx, desiredDeployment, metav1.UpdateOptions{})
7884
if err != nil {
@@ -94,6 +100,16 @@ func (c *Reconciler) checkAndUpdateDeployment(ctx context.Context, rev *v1.Revis
94100
return d, nil
95101
}
96102

103+
// mergeMetadata merges Knative-managed metadata with external metadata.
104+
// Knative controls knative.dev keys and the app label; other keys are preserved from current.
105+
func mergeMetadata(desired, current map[string]string) map[string]string {
106+
isKnativeKey := func(key string) bool {
107+
return key == resources.AppLabelKey || strings.Contains(key, "knative.dev/")
108+
}
109+
external := kmap.Filter(current, isKnativeKey)
110+
return kmap.Union(desired, external)
111+
}
112+
97113
func (c *Reconciler) createImageCache(ctx context.Context, rev *v1.Revision, containerName, imageDigest string) (*caching.Image, error) {
98114
image := resources.MakeImageCache(rev, containerName, imageDigest)
99115
return c.cachingclient.CachingV1alpha1().Images(image.Namespace).Create(ctx, image, metav1.CreateOptions{})
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
Copyright 2025 The Knative Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package revision
18+
19+
import (
20+
"testing"
21+
22+
"github.com/google/go-cmp/cmp"
23+
)
24+
25+
func TestMergeMetadata(t *testing.T) {
26+
tests := []struct {
27+
name string
28+
desired map[string]string
29+
current map[string]string
30+
want map[string]string
31+
}{
32+
{
33+
name: "preserve external annotations",
34+
desired: map[string]string{"serving.knative.dev/creator": "kubernetes-admin"},
35+
current: map[string]string{"kubectl.kubernetes.io/restartedAt": "2025-11-27T12:14:41+01:00"},
36+
want: map[string]string{"serving.knative.dev/creator": "kubernetes-admin", "kubectl.kubernetes.io/restartedAt": "2025-11-27T12:14:41+01:00"},
37+
},
38+
{
39+
name: "knative annotations from desired win",
40+
desired: map[string]string{"serving.knative.dev/lastModifier": "kubernetes-admin"},
41+
current: map[string]string{"serving.knative.dev/lastModifier": "old-user"},
42+
want: map[string]string{"serving.knative.dev/lastModifier": "kubernetes-admin"},
43+
},
44+
{
45+
name: "delete knative annotations not in desired",
46+
desired: map[string]string{"serving.knative.dev/creator": "kubernetes-admin"},
47+
current: map[string]string{"serving.knative.dev/creator": "kubernetes-admin", "autoscaling.knative.dev/min-scale": "2"},
48+
want: map[string]string{"serving.knative.dev/creator": "kubernetes-admin"},
49+
},
50+
{
51+
name: "app label from desired wins",
52+
desired: map[string]string{"app": "new-revision"},
53+
current: map[string]string{"app": "old-revision"},
54+
want: map[string]string{"app": "new-revision"},
55+
},
56+
{
57+
name: "mixed knative and external metadata",
58+
desired: map[string]string{"autoscaling.knative.dev/min-scale": "1", "app": "my-revision"},
59+
current: map[string]string{"autoscaling.knative.dev/target-burst-capacity": "0", "deployment.kubernetes.io/revision": "2", "kubectl.kubernetes.io/restartedAt": "2025-11-27T12:14:41+01:00"},
60+
want: map[string]string{"autoscaling.knative.dev/min-scale": "1", "app": "my-revision", "deployment.kubernetes.io/revision": "2", "kubectl.kubernetes.io/restartedAt": "2025-11-27T12:14:41+01:00"},
61+
},
62+
}
63+
64+
for _, tt := range tests {
65+
t.Run(tt.name, func(t *testing.T) {
66+
got := mergeMetadata(tt.desired, tt.current)
67+
if diff := cmp.Diff(tt.want, got); diff != "" {
68+
t.Errorf("mergeMetadata() mismatch (-want +got):\n%s", diff)
69+
}
70+
})
71+
}
72+
}

pkg/reconciler/revision/table_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,19 @@ func TestReconcile(t *testing.T) {
210210
Object: deploy(t, "foo", "fix-containers"),
211211
}},
212212
Key: "foo/fix-containers",
213+
}, {
214+
Name: "preserve external deployment and template annotations and labels",
215+
Objects: []runtime.Object{
216+
Revision("foo", "preserve-annotations",
217+
WithLogURL, allUnknownConditions, withDefaultContainerStatuses(), WithRevisionObservedGeneration(1)),
218+
pa("foo", "preserve-annotations", WithReachabilityUnknown),
219+
addDeploymentMetadata(deploy(t, "foo", "preserve-annotations"), true),
220+
image("foo", "preserve-annotations"),
221+
},
222+
WantUpdates: []clientgotesting.UpdateActionImpl{{
223+
Object: addDeploymentMetadata(deploy(t, "foo", "preserve-annotations"), false),
224+
}},
225+
Key: "foo/preserve-annotations",
213226
}, {
214227
Name: "failure updating deployment",
215228
// Test that we handle an error updating the deployment properly.
@@ -945,6 +958,25 @@ func changeContainers(deploy *appsv1.Deployment) *appsv1.Deployment {
945958
return deploy
946959
}
947960

961+
func addDeploymentMetadata(deploy *appsv1.Deployment, addInternalLabels bool) *appsv1.Deployment {
962+
setValue := func(m map[string]string, scope string) {
963+
m["external.io/"+scope] = "external-" + scope
964+
if addInternalLabels {
965+
m[resources.AppLabelKey] = "app"
966+
m["internal.knative.dev/foo"] = "bar"
967+
}
968+
}
969+
// Deployment-level metadata
970+
setValue(deploy.Annotations, "annotation")
971+
setValue(deploy.Labels, "label")
972+
973+
// Template-level metadata (e.g. from kubectl restart)
974+
setValue(deploy.Spec.Template.Annotations, "template-annotation")
975+
setValue(deploy.Spec.Template.Labels, "template-label")
976+
977+
return deploy
978+
}
979+
948980
func withDefaultContainerStatuses() RevisionOption {
949981
return func(r *v1.Revision) {
950982
r.Status.ContainerStatuses = []v1.ContainerStatus{{

0 commit comments

Comments
 (0)