Skip to content

Commit 9a7fd36

Browse files
committed
fix(ipv6): do not cache an answer when every family probe is rejected
Probing each family on its own merit lost the control the previous shape had. A cluster serves at least one family, so two rejections mean something refuses every Service, not that the cluster has no ranges. The detector treated that as a resolved answer of "no families" and kept it for the life of the process, so an admission webhook that was later fixed left the operator on the wrong answer until a restart. Signed-off-by: Bence Csati <bence.csati@axoflow.com>
1 parent f8da844 commit 9a7fd36

3 files changed

Lines changed: 18 additions & 4 deletions

File tree

pkg/resources/ipfamily/detector.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ func (d *Detector) Families(ctx context.Context, namespace string) []corev1.IPFa
6363
return nil
6464
}
6565

66+
// No cluster serves neither family, so something rejects every Service and the answer is not
67+
// ours to cache. An admission webhook doing that would otherwise pin nil until a restart.
68+
if !hasIPv6 && !hasIPv4 {
69+
d.log.Info("every IP family probe was rejected, leaving the IP families to the cluster for now")
70+
return nil
71+
}
72+
6673
switch {
6774
case hasIPv6 && hasIPv4:
6875
d.families = []corev1.IPFamily{corev1.IPv6Protocol, corev1.IPv4Protocol}
@@ -75,8 +82,6 @@ func (d *Detector) Families(ctx context.Context, namespace string) []corev1.IPFa
7582
return d.families
7683
}
7784

78-
// An admission webhook that rejects every Service is indistinguishable from a missing range, so an
79-
// error here leaves the answer unresolved rather than caching a guess.
8085
func (d *Detector) allocatable(ctx context.Context, namespace string, family corev1.IPFamily) (bool, error) {
8186
err := d.dryRun(ctx, namespace, family)
8287
if err == nil {

pkg/resources/ipfamily/detector_test.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ func TestFamilies(t *testing.T) {
5353
expected: nil,
5454
resolved: true,
5555
},
56+
{
57+
name: "a webhook rejecting every service leaves the answer unknown",
58+
results: map[corev1.IPFamily]error{
59+
corev1.IPv6Protocol: invalidFamilyErr(),
60+
corev1.IPv4Protocol: invalidFamilyErr(),
61+
},
62+
expected: nil,
63+
resolved: false,
64+
},
5665
{
5766
name: "a denied probe leaves the answer unknown",
5867
results: map[corev1.IPFamily]error{corev1.IPv6Protocol: apierrors.NewForbidden(schema.GroupResource{Resource: "services"}, "probe", errors.New("nope"))},

pkg/sdk/logging/api/v1beta1/common_types.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,8 @@ func EnableIPv6Options(serviceSpec *corev1.ServiceSpec, clusterFamilies []corev1
228228
// PreserveAllocatedIPFamilies pins ipFamilies and ipFamilyPolicy to what the API server allocated.
229229
// A change to an existing Service's primary family is rejected as invalid, not as immutable, so the
230230
// recreate fallback never fires. It reports whether it overrode a single-stack policy the caller
231-
// asked for, which the caller should surface: the request is silently not carried out.
232-
func PreserveAllocatedIPFamilies(desired *corev1.ServiceSpec, current corev1.ServiceSpec) (overrodePolicy bool) {
231+
// asked for, so the caller can say that the request was not carried out.
232+
func PreserveAllocatedIPFamilies(desired *corev1.ServiceSpec, current corev1.ServiceSpec) bool {
233233
allocated := len(current.ClusterIPs) > 0 && current.ClusterIPs[0] != corev1.ClusterIPNone
234234
if len(current.IPFamilies) == 0 || !allocated {
235235
return false

0 commit comments

Comments
 (0)