Skip to content

Commit 780d593

Browse files
committed
feat: create per-tag KIngress resources instead of a single KIngress per Route
Instead of consolidating all traffic tags into one KIngress, the Route reconciler now creates individual KIngress resources for each traffic tag. Each per-tag KIngress is labeled with networking.internal.knative.dev/tag to identify which tag it serves. This enables independent lifecycle management and status tracking per tag. Signed-off-by: kahirokunn <okinakahiro@gmail.com>
1 parent cff5211 commit 780d593

8 files changed

Lines changed: 644 additions & 710 deletions

File tree

pkg/reconciler/route/reconcile_resources.go

Lines changed: 123 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -43,69 +43,134 @@ import (
4343
v1 "knative.dev/serving/pkg/apis/serving/v1"
4444
"knative.dev/serving/pkg/reconciler/route/config"
4545
"knative.dev/serving/pkg/reconciler/route/resources"
46-
"knative.dev/serving/pkg/reconciler/route/resources/names"
4746
"knative.dev/serving/pkg/reconciler/route/traffic"
4847
)
4948

50-
func (c *Reconciler) reconcileIngress(
49+
// tagLabelKey is the label key for per-tag Ingress resources.
50+
const tagLabelKey = networking.GroupName + "/tag"
51+
52+
func (c *Reconciler) reconcileIngresses(
5153
ctx context.Context, r *v1.Route, tc *traffic.Config,
5254
tls []netv1alpha1.IngressTLS,
5355
ingressClass string,
5456
acmeChallenges ...netv1alpha1.HTTP01Challenge,
55-
) (*netv1alpha1.Ingress, *traffic.Rollout, error) {
57+
) ([]*netv1alpha1.Ingress, *traffic.Rollout, error) {
5658
recorder := controller.GetEventRecorder(ctx)
57-
var effectiveRO *traffic.Rollout
5859

59-
ingress, err := c.ingressLister.Ingresses(r.Namespace).Get(names.Ingress(r))
60-
if apierrs.IsNotFound(err) {
61-
desired, err := resources.MakeIngress(ctx, r, tc, tls, ingressClass, acmeChallenges...)
62-
if err != nil {
60+
// Build desired ingresses (without rollout) to determine names.
61+
desired, err := resources.MakeIngress(ctx, r, tc, tls, ingressClass, acmeChallenges...)
62+
if err != nil {
63+
return nil, nil, err
64+
}
65+
66+
// Collect previous rollouts from existing per-tag ingresses and check readiness.
67+
desiredNames := sets.New[string]()
68+
prevRO := &traffic.Rollout{}
69+
existingIngresses := map[string]*netv1alpha1.Ingress{}
70+
allExistingReady := true
71+
hasExisting := false
72+
73+
for _, d := range desired {
74+
desiredNames.Insert(d.Name)
75+
existing, err := c.ingressLister.Ingresses(r.Namespace).Get(d.Name)
76+
if apierrs.IsNotFound(err) {
77+
allExistingReady = false
78+
continue
79+
} else if err != nil {
6380
return nil, nil, err
6481
}
65-
ingress, err = c.netclient.NetworkingV1alpha1().Ingresses(desired.Namespace).Create(ctx, desired, metav1.CreateOptions{})
66-
if err != nil {
67-
recorder.Eventf(r, corev1.EventTypeWarning, "CreationFailed", "Failed to create Ingress: %v", err)
68-
return nil, nil, fmt.Errorf("failed to create Ingress: %w", err)
82+
hasExisting = true
83+
existingIngresses[d.Name] = existing
84+
85+
tagRO := deserializeRollout(ctx, existing.Annotations[networking.RolloutAnnotationKey])
86+
if tagRO != nil {
87+
prevRO.Configurations = append(prevRO.Configurations, tagRO.Configurations...)
6988
}
7089

71-
recorder.Eventf(r, corev1.EventTypeNormal, "Created", "Created Ingress %q", ingress.GetName())
72-
return ingress, tc.BuildRollout(), nil
73-
} else if err != nil {
74-
return nil, nil, err
75-
} else {
76-
// Ingress exists. We need to compute the rollout spec diff.
77-
effectiveRO = c.reconcileRollout(ctx, r, tc, ingress)
78-
desired, err := resources.MakeIngressWithRollout(ctx, r, tc, effectiveRO,
79-
tls, ingressClass, acmeChallenges...)
80-
if err != nil {
81-
return nil, nil, err
90+
if !existing.IsReady() {
91+
allExistingReady = false
8292
}
93+
}
94+
95+
// Compute the effective rollout.
96+
var effectiveRO *traffic.Rollout
97+
if !hasExisting {
98+
effectiveRO = tc.BuildRollout()
99+
} else {
100+
effectiveRO = c.reconcileRolloutFromIngresses(ctx, r, tc, prevRO, allExistingReady)
101+
}
102+
103+
// Rebuild desired with the effective rollout.
104+
desired, err = resources.MakeIngressWithRollout(ctx, r, tc, effectiveRO, tls, ingressClass, acmeChallenges...)
105+
if err != nil {
106+
return nil, nil, err
107+
}
83108

84-
if !equality.Semantic.DeepEqual(ingress.Spec, desired.Spec) ||
85-
!equality.Semantic.DeepEqual(ingress.Annotations, desired.Annotations) ||
86-
!equality.Semantic.DeepEqual(ingress.Labels, desired.Labels) {
87-
// It is notable that one reason for differences here may be defaulting.
88-
// When that is the case, the Update will end up being a nop because the
89-
// webhook will bring them into alignment and no new reconciliation will occur.
90-
// Also, compare annotation and label in case ingress.Class or parent route's labels
91-
// is updated.
92-
93-
// Don't modify the informers copy.
94-
origin := ingress.DeepCopy()
95-
origin.Spec = desired.Spec
96-
origin.Annotations = desired.Annotations
97-
origin.Labels = desired.Labels
98-
99-
updated, err := c.netclient.NetworkingV1alpha1().Ingresses(origin.Namespace).Update(
100-
ctx, origin, metav1.UpdateOptions{})
109+
// Create or update each desired per-tag ingress.
110+
var result []*netv1alpha1.Ingress
111+
for _, d := range desired {
112+
existing, ok := existingIngresses[d.Name]
113+
if !ok {
114+
created, err := c.netclient.NetworkingV1alpha1().Ingresses(d.Namespace).Create(ctx, d, metav1.CreateOptions{})
101115
if err != nil {
102-
return nil, nil, fmt.Errorf("failed to update Ingress: %w", err)
116+
recorder.Eventf(r, corev1.EventTypeWarning, "CreationFailed", "Failed to create Ingress: %v", err)
117+
return nil, nil, fmt.Errorf("failed to create Ingress: %w", err)
118+
}
119+
recorder.Eventf(r, corev1.EventTypeNormal, "Created", "Created Ingress %q", created.GetName())
120+
result = append(result, created)
121+
} else {
122+
if !equality.Semantic.DeepEqual(existing.Spec, d.Spec) ||
123+
!equality.Semantic.DeepEqual(existing.Annotations, d.Annotations) ||
124+
!equality.Semantic.DeepEqual(existing.Labels, d.Labels) {
125+
origin := existing.DeepCopy()
126+
origin.Spec = d.Spec
127+
origin.Annotations = d.Annotations
128+
origin.Labels = d.Labels
129+
130+
updated, err := c.netclient.NetworkingV1alpha1().Ingresses(origin.Namespace).Update(
131+
ctx, origin, metav1.UpdateOptions{})
132+
if err != nil {
133+
return nil, nil, fmt.Errorf("failed to update Ingress: %w", err)
134+
}
135+
result = append(result, updated)
136+
} else {
137+
result = append(result, existing)
103138
}
104-
return updated, effectiveRO, nil
105139
}
106140
}
107141

108-
return ingress, effectiveRO, err
142+
// Delete orphaned ingresses (tags that no longer exist).
143+
if err := c.deleteOrphanedIngresses(ctx, r, desiredNames); err != nil {
144+
return nil, nil, err
145+
}
146+
147+
return result, effectiveRO, nil
148+
}
149+
150+
func (c *Reconciler) deleteOrphanedIngresses(ctx context.Context, r *v1.Route, desiredNames sets.Set[string]) error {
151+
routeLabelSelector := labels.SelectorFromSet(labels.Set{serving.RouteLabelKey: r.Name})
152+
allIngresses, err := c.ingressLister.Ingresses(r.Namespace).List(routeLabelSelector)
153+
if err != nil {
154+
return fmt.Errorf("failed to fetch existing ingresses: %w", err)
155+
}
156+
157+
recorder := controller.GetEventRecorder(ctx)
158+
for _, ing := range allIngresses {
159+
if desiredNames.Has(ing.Name) {
160+
continue
161+
}
162+
if !metav1.IsControlledBy(ing, r) {
163+
continue
164+
}
165+
if err := c.netclient.NetworkingV1alpha1().Ingresses(r.Namespace).Delete(
166+
ctx, ing.Name, metav1.DeleteOptions{}); err != nil && !apierrs.IsNotFound(err) {
167+
recorder.Eventf(r, corev1.EventTypeWarning, "DeleteFailed",
168+
"Failed to delete orphaned Ingress %q: %v", ing.Name, err)
169+
return fmt.Errorf("failed to delete orphaned Ingress: %w", err)
170+
}
171+
recorder.Eventf(r, corev1.EventTypeNormal, "Deleted", "Deleted orphaned Ingress %q", ing.Name)
172+
}
173+
return nil
109174
}
110175

111176
func (c *Reconciler) deleteOrphanedServices(ctx context.Context, r *v1.Route, activeServices []resources.ServicePair) error {
@@ -215,13 +280,18 @@ func (c *Reconciler) reconcilePlaceholderServices(ctx context.Context, route *v1
215280
return services, nil
216281
}
217282

218-
func (c *Reconciler) updatePlaceholderServices(ctx context.Context, route *v1.Route, pairs []resources.ServicePair, ingress *netv1alpha1.Ingress) error {
283+
func (c *Reconciler) updatePlaceholderServices(ctx context.Context, route *v1.Route, pairs []resources.ServicePair, ingressByTag map[string]*netv1alpha1.Ingress) error {
219284
logger := logging.FromContext(ctx)
220285
ns := route.Namespace
221286

222287
eg, egCtx := errgroup.WithContext(ctx)
223288
for _, from := range pairs {
224289
eg.Go(func() error {
290+
ingress, ok := ingressByTag[from.Tag]
291+
if !ok {
292+
logger.Warnw("No ingress found for tag, skipping placeholder update", zap.String("tag", from.Tag))
293+
return nil
294+
}
225295
to, err := resources.MakeK8sService(egCtx, route, from.Tag, ingress, resources.IsClusterLocalService(from.Service))
226296
if err != nil {
227297
// Loadbalancer not ready, no need to update.
@@ -328,9 +398,9 @@ func deserializeRollout(ctx context.Context, ro string) *traffic.Rollout {
328398
return r
329399
}
330400

331-
func (c *Reconciler) reconcileRollout(
401+
func (c *Reconciler) reconcileRolloutFromIngresses(
332402
ctx context.Context, r *v1.Route, tc *traffic.Config,
333-
ingress *netv1alpha1.Ingress,
403+
prevRO *traffic.Rollout, allIngressesReady bool,
334404
) *traffic.Rollout {
335405
cfg := config.FromContext(ctx)
336406

@@ -349,18 +419,18 @@ func (c *Reconciler) reconcileRollout(
349419
logger := logging.FromContext(ctx).Desugar().With(
350420
zap.Int("durationSecs", rd))
351421
logger.Debug("Rollout is enabled. Stepping from previous state.")
352-
// Get the previous rollout state from the annotation.
353-
// If it's corrupt, inexistent, or otherwise incorrect,
354-
// the prevRO will be just nil rollout.
355-
prevRO := deserializeRollout(ctx,
356-
ingress.Annotations[networking.RolloutAnnotationKey])
422+
423+
// prevRO was assembled by merging per-tag rollouts from individual ingresses.
424+
if prevRO == nil || len(prevRO.Configurations) == 0 {
425+
prevRO = nil
426+
}
357427

358428
// And recompute the rollout state.
359429
now := c.clock.Now().UnixNano()
360430

361-
// Now check if the ingress status changed from not ready to ready.
431+
// Now check if all ingresses transitioned from not ready to ready.
362432
rtView := r.Status.GetCondition(v1.RouteConditionIngressReady)
363-
if prevRO != nil && ingress.IsReady() && !rtView.IsTrue() {
433+
if prevRO != nil && allIngressesReady && !rtView.IsTrue() {
364434
logger.Debug("Observing Ingress not-ready to ready switch condition for rollout")
365435
prevRO.ObserveReady(ctx, now, float64(rd))
366436
}

0 commit comments

Comments
 (0)