cmd/k8s-operator: ensure status update errors are displayed to users (#16251)

Updates#cleanup

Signed-off-by: Irbe Krumina <irbe@tailscale.com>

cmd/k8s-operator/ingress_test.go

@@ -427,7 +427,7 @@ func TestIngressLetsEncryptStaging(t *testing.T) {
 
 	pcLEStaging, pcLEStagingFalse, pcOther := proxyClassesForLEStagingTest()
 
-	testCases := testCasesForLEStagingTests(pcLEStaging, pcLEStagingFalse, pcOther)
+	testCases := testCasesForLEStagingTests()
 
 	for _, tt := range testCases {
 		t.Run(tt.name, func(t *testing.T) {

cmd/k8s-operator/nameserver.go

@@ -7,6 +7,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"slices"
 	"strings"
@@ -14,7 +15,6 @@ import (
 
 	_ "embed"
 
-	"github.com/pkg/errors"
 	"go.uber.org/zap"
 	xslices "golang.org/x/exp/slices"
 	appsv1 "k8s.io/api/apps/v1"
@@ -106,7 +106,7 @@ func (a *NameserverReconciler) Reconcile(ctx context.Context, req reconcile.Requ
 		if !apiequality.Semantic.DeepEqual(oldCnStatus, &dnsCfg.Status) {
 			// An error encountered here should get returned by the Reconcile function.
 			if updateErr := a.Client.Status().Update(ctx, dnsCfg); updateErr != nil {
-				err = errors.Wrap(err, updateErr.Error())
+				err = errors.Join(err, updateErr)
 			}
 		}
 		return res, err

cmd/k8s-operator/proxygroup.go

@@ -9,13 +9,13 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"slices"
 	"strings"
 	"sync"
 
-	"github.com/pkg/errors"
 	"go.uber.org/zap"
 	xslices "golang.org/x/exp/slices"
 	appsv1 "k8s.io/api/apps/v1"
@@ -122,7 +122,7 @@ func (r *ProxyGroupReconciler) Reconcile(ctx context.Context, req reconcile.Requ
 		if !apiequality.Semantic.DeepEqual(oldPGStatus, &pg.Status) {
 			// An error encountered here should get returned by the Reconcile function.
 			if updateErr := r.Client.Status().Update(ctx, pg); updateErr != nil {
-				err = errors.Wrap(err, updateErr.Error())
+				err = errors.Join(err, updateErr)
 			}
 		}
 		return reconcile.Result{}, err

cmd/k8s-operator/proxygroup_test.go

@@ -257,10 +257,12 @@ func TestProxyGroupTypes(t *testing.T) {
 		},
 		Spec: tsapi.ProxyClassSpec{},
 	}
+	// Passing ProxyGroup as status subresource is a way to get around fake
+	// client's limitations for updating resource statuses.
 	fc := fake.NewClientBuilder().
 		WithScheme(tsapi.GlobalScheme).
 		WithObjects(pc).
-		WithStatusSubresource(pc).
+		WithStatusSubresource(pc, &tsapi.ProxyGroup{}).
 		Build()
 	mustUpdateStatus(t, fc, "", pc.Name, func(p *tsapi.ProxyClass) {
 		p.Status.Conditions = []metav1.Condition{{
@@ -450,6 +452,7 @@ func TestProxyGroupTypes(t *testing.T) {
 func TestIngressAdvertiseServicesConfigPreserved(t *testing.T) {
 	fc := fake.NewClientBuilder().
 		WithScheme(tsapi.GlobalScheme).
+		WithStatusSubresource(&tsapi.ProxyGroup{}).
 		Build()
 	reconciler := &ProxyGroupReconciler{
 		tsNamespace: tsNamespace,
@@ -693,7 +696,7 @@ func TestProxyGroupLetsEncryptStaging(t *testing.T) {
 		pgType tsapi.ProxyGroupType
 	}
 	pcLEStaging, pcLEStagingFalse, pcOther := proxyClassesForLEStagingTest()
-	sharedTestCases := testCasesForLEStagingTests(pcLEStaging, pcLEStagingFalse, pcOther)
+	sharedTestCases := testCasesForLEStagingTests()
 	var tests []proxyGroupLETestCase
 	for _, tt := range sharedTestCases {
 		tests = append(tests, proxyGroupLETestCase{
@@ -715,9 +718,20 @@ func TestProxyGroupLetsEncryptStaging(t *testing.T) {
 			builder := fake.NewClientBuilder().
 				WithScheme(tsapi.GlobalScheme)
 
+			pg := &tsapi.ProxyGroup{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test",
+				},
+				Spec: tsapi.ProxyGroupSpec{
+					Type:       tt.pgType,
+					Replicas:   ptr.To[int32](1),
+					ProxyClass: tt.proxyClassPerResource,
+				},
+			}
+
 			// Pre-populate the fake client with ProxyClasses.
-			builder = builder.WithObjects(pcLEStaging, pcLEStagingFalse, pcOther).
-				WithStatusSubresource(pcLEStaging, pcLEStagingFalse, pcOther)
+			builder = builder.WithObjects(pcLEStaging, pcLEStagingFalse, pcOther, pg).
+				WithStatusSubresource(pcLEStaging, pcLEStagingFalse, pcOther, pg)
 
 			fc := builder.Build()
 
@@ -730,19 +744,6 @@ func TestProxyGroupLetsEncryptStaging(t *testing.T) {
 				setProxyClassReady(t, fc, cl, name)
 			}
 
-			// Create ProxyGroup
-			pg := &tsapi.ProxyGroup{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "test",
-				},
-				Spec: tsapi.ProxyGroupSpec{
-					Type:       tt.pgType,
-					Replicas:   ptr.To[int32](1),
-					ProxyClass: tt.proxyClassPerResource,
-				},
-			}
-			mustCreate(t, fc, pg)
-
 			reconciler := &ProxyGroupReconciler{
 				tsNamespace:       tsNamespace,
 				proxyImage:        testProxyImage,
@@ -783,7 +784,7 @@ type leStagingTestCase struct {
 
 // Shared test cases for LE staging endpoint configuration for ProxyGroup and
 // non-HA Ingress.
-func testCasesForLEStagingTests(pcLEStaging, pcLEStagingFalse, pcOther *tsapi.ProxyClass) []leStagingTestCase {
+func testCasesForLEStagingTests() []leStagingTestCase {
 	return []leStagingTestCase{
 		{
 			name:                  "with_staging_proxyclass",