From 9d5fea2ed36c6ed0f388e8eda4c377132b55c86e Mon Sep 17 00:00:00 2001 From: Damian Sawicki Date: Fri, 7 Apr 2023 13:18:07 +0000 Subject: [PATCH] Emit event on Description-only healthcheck update --- pkg/backends/integration_test.go | 3 ++- pkg/backends/syncer_test.go | 3 ++- pkg/controller/controller.go | 4 +++- pkg/healthchecks/healthcheck.go | 8 +++++++ pkg/healthchecks/healthchecks.go | 11 +++++++-- pkg/healthchecks/healthchecks_test.go | 30 ++++++++++++++++++------- pkg/translator/healthchecks.go | 32 +++++++++++++++++++++++++++ 7 files changed, 78 insertions(+), 13 deletions(-) diff --git a/pkg/backends/integration_test.go b/pkg/backends/integration_test.go index d3c961f661..ff664992af 100644 --- a/pkg/backends/integration_test.go +++ b/pkg/backends/integration_test.go @@ -26,6 +26,7 @@ import ( "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/mock" compute "google.golang.org/api/compute/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/record" "k8s.io/cloud-provider-gcp/providers/gce" "k8s.io/ingress-gce/pkg/annotations" "k8s.io/ingress-gce/pkg/healthchecks" @@ -42,7 +43,7 @@ type Jig struct { } func newTestJig(fakeGCE *gce.Cloud) *Jig { - fakeHealthChecks := healthchecks.NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + fakeHealthChecks := healthchecks.NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) fakeBackendPool := NewPool(fakeGCE, defaultNamer) fakeIGs := instancegroups.NewEmptyFakeInstanceGroups() diff --git a/pkg/backends/syncer_test.go b/pkg/backends/syncer_test.go index 88432dbc24..559f2c38f6 100644 --- a/pkg/backends/syncer_test.go +++ b/pkg/backends/syncer_test.go @@ -33,6 +33,7 @@ import ( networkingv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/tools/record" "k8s.io/cloud-provider-gcp/providers/gce" "k8s.io/ingress-gce/pkg/annotations" "k8s.io/ingress-gce/pkg/backends/features" @@ -137,7 +138,7 @@ var ( ) func newTestSyncer(fakeGCE *gce.Cloud) *backendSyncer { - fakeHealthChecks := healthchecks.NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + fakeHealthChecks := healthchecks.NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) fakeBackendPool := NewPool(fakeGCE, defaultNamer) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 64d58f8ead..3e47485580 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -114,7 +114,9 @@ func NewLoadBalancerController( Interface: ctx.KubeClient.CoreV1().Events(""), }) - healthChecker := healthchecks.NewHealthChecker(ctx.Cloud, ctx.HealthCheckPath, ctx.DefaultBackendSvcPort.ID.Service) + eventRecorder := ctx.Recorder(ctx.Namespace) + + healthChecker := healthchecks.NewHealthChecker(ctx.Cloud, ctx.HealthCheckPath, ctx.DefaultBackendSvcPort.ID.Service, eventRecorder) backendPool := backends.NewPool(ctx.Cloud, ctx.ClusterNamer) lbc := LoadBalancerController{ diff --git a/pkg/healthchecks/healthcheck.go b/pkg/healthchecks/healthcheck.go index 2c605c25f3..19801d2c33 100644 --- a/pkg/healthchecks/healthcheck.go +++ b/pkg/healthchecks/healthcheck.go @@ -33,6 +33,14 @@ type fieldDiffs struct { func (c *fieldDiffs) add(field, oldv, newv string) { c.f = append(c.f, fmt.Sprintf("%s:%s -> %s", field, oldv, newv)) } +func (c *fieldDiffs) has(field string) bool { + for _, s := range c.f { + if strings.HasPrefix(s, field+":") { + return true + } + } + return false +} func (c *fieldDiffs) String() string { return strings.Join(c.f, ", ") } func (c *fieldDiffs) hasDiff() bool { return len(c.f) > 0 } func (c *fieldDiffs) size() int64 { return int64(len(c.f)) } diff --git a/pkg/healthchecks/healthchecks.go b/pkg/healthchecks/healthchecks.go index cb7ae56451..22d21bf568 100644 --- a/pkg/healthchecks/healthchecks.go +++ b/pkg/healthchecks/healthchecks.go @@ -25,6 +25,7 @@ import ( computealpha "google.golang.org/api/compute/v0.alpha" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" "k8s.io/cloud-provider-gcp/providers/gce" backendconfigv1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1" "k8s.io/ingress-gce/pkg/composite" @@ -43,13 +44,14 @@ type HealthChecks struct { // This is a workaround which allows us to not have to maintain // a separate health checker for the default backend. defaultBackendSvc types.NamespacedName + eventRecorder record.EventRecorder } // NewHealthChecker creates a new health checker. // cloud: the cloud object implementing SingleHealthCheck. // defaultHealthCheckPath: is the HTTP path to use for health checks. -func NewHealthChecker(cloud HealthCheckProvider, healthCheckPath string, defaultBackendSvc types.NamespacedName) *HealthChecks { - return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc} +func NewHealthChecker(cloud HealthCheckProvider, healthCheckPath string, defaultBackendSvc types.NamespacedName, eventRecorder record.EventRecorder) *HealthChecks { + return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc, eventRecorder} } // new returns a *HealthCheck with default settings and specified port/protocol @@ -142,6 +144,11 @@ func (h *HealthChecks) sync(hc *translator.HealthCheck, bchcc *backendconfigv1.H changes := calculateDiff(filter(existingHC), filter(hc), bchcc) if changes.hasDiff() { klog.V(2).Infof("Health check %q needs update (%s)", existingHC.Name, changes) + if flags.F.EnableUpdateCustomHealthCheckDescription && changes.size() == 1 && changes.has("Description") { + message := fmt.Sprintf("Healthcheck will be updated and the only field updated is Description.\nOld: %+v\nNew: %+v\nDiff: %+v", existingHC, hc, changes) + h.eventRecorder.Event( + hc, "Warning", "HealthcheckDescriptionUpdate", message) + } err := h.update(hc) if err != nil { klog.Errorf("Health check %q update error: %v", existingHC.Name, err) diff --git a/pkg/healthchecks/healthchecks_test.go b/pkg/healthchecks/healthchecks_test.go index f23e7637b1..0d7a99d57e 100644 --- a/pkg/healthchecks/healthchecks_test.go +++ b/pkg/healthchecks/healthchecks_test.go @@ -21,7 +21,9 @@ import ( "fmt" "net/http" "reflect" + "strings" "testing" + "time" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" @@ -34,6 +36,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/tools/record" "k8s.io/cloud-provider-gcp/providers/gce" "k8s.io/ingress-gce/pkg/annotations" backendconfigv1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1" @@ -111,7 +114,7 @@ func init() { func TestHealthCheckAdd(t *testing.T) { fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) - healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) sp := &utils.ServicePort{NodePort: 80, Protocol: annotations.ProtocolHTTP, NEGEnabled: false, BackendNamer: testNamer} _, err := healthChecks.SyncServicePort(sp, nil) @@ -149,7 +152,7 @@ func TestHealthCheckAdd(t *testing.T) { func TestHealthCheckAddExisting(t *testing.T) { fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) - healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) // HTTP // Manually insert a health check @@ -221,7 +224,7 @@ func TestHealthCheckAddExisting(t *testing.T) { func TestHealthCheckDelete(t *testing.T) { fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) - healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) // Create HTTP HC for 1234 hc := translator.DefaultHealthCheck(1234, annotations.ProtocolHTTP) @@ -251,7 +254,7 @@ func TestHealthCheckDelete(t *testing.T) { func TestHTTP2HealthCheckDelete(t *testing.T) { fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) - healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) // Create HTTP2 HC for 1234 hc := translator.DefaultHealthCheck(1234, annotations.ProtocolHTTP2) @@ -278,7 +281,7 @@ func TestHTTP2HealthCheckDelete(t *testing.T) { func TestRegionalHealthCheckDelete(t *testing.T) { fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) - healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) hc := healthChecks.new( utils.ServicePort{ @@ -330,7 +333,7 @@ func TestHealthCheckUpdate(t *testing.T) { (fakeGCE.Compute().(*cloud.MockGCE)).MockAlphaHealthChecks.UpdateHook = mock.UpdateAlphaHealthCheckHook (fakeGCE.Compute().(*cloud.MockGCE)).MockBetaHealthChecks.UpdateHook = mock.UpdateBetaHealthCheckHook - healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) // HTTP // Manually insert a health check @@ -465,7 +468,8 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) { (fakeGCE.Compute().(*cloud.MockGCE)).MockAlphaHealthChecks.UpdateHook = mock.UpdateAlphaHealthCheckHook (fakeGCE.Compute().(*cloud.MockGCE)).MockBetaHealthChecks.UpdateHook = mock.UpdateBetaHealthCheckHook - healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + fakeRecorder := record.NewFakeRecorder(1) + healthChecks := NewHealthChecker(fakeGCE, "/", defaultBackendSvc, fakeRecorder) _, err := healthChecks.SyncServicePort(defaultSP, nil) if err != nil { @@ -517,6 +521,16 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) { if !reflect.DeepEqual(outputBCHC, outputBCHCWithFlag) { t.Fatalf("Compute healthcheck is:\n%s, want:\n%s", pretty.Sprint(outputBCHC), pretty.Sprint(outputBCHCWithFlag)) } + + select { + case output := <-fakeRecorder.Events: + if !strings.HasPrefix(output, "Warning HealthcheckDescriptionUpdate") { + t.Fatalf("Incorrect event emitted on healthcheck update: %s.", output) + } + case <-time.After(2 * time.Second): + t.Fatalf("Expected event.") + } + } func TestVersion(t *testing.T) { @@ -1342,7 +1356,7 @@ func TestSyncServicePort(t *testing.T) { tc.setup(mock) } - hcs := NewHealthChecker(fakeGCE, "/", defaultBackendSvc) + hcs := NewHealthChecker(fakeGCE, "/", defaultBackendSvc, record.NewFakeRecorder(0)) gotSelfLink, err := hcs.SyncServicePort(tc.sp, tc.probe) if gotErr := err != nil; gotErr != tc.wantErr { diff --git a/pkg/translator/healthchecks.go b/pkg/translator/healthchecks.go index caa541559c..591aefdd1e 100644 --- a/pkg/translator/healthchecks.go +++ b/pkg/translator/healthchecks.go @@ -17,6 +17,7 @@ limitations under the License. package translator import ( + "encoding/json" "errors" "fmt" "strings" @@ -27,6 +28,8 @@ import ( computebeta "google.golang.org/api/compute/v0.beta" "google.golang.org/api/compute/v1" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/ingress-gce/pkg/annotations" backendconfigv1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1" "k8s.io/ingress-gce/pkg/flags" @@ -338,3 +341,32 @@ func ApplyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) { hc.Description = DescriptionForHealthChecksFromReadinessProbe } + +// GetObjectKind implements runtime.Object +func (*HealthCheck) GetObjectKind() schema.ObjectKind { + panic("unimplemented") +} + +func copyViaJSON(dst, src interface{}) error { + var err error + bytes, err := json.Marshal(src) + if err != nil { + return err + } + return json.Unmarshal(bytes, dst) +} + +func DeepCopyTranslatorHealthCheck(src *HealthCheck) *HealthCheck { + dst := &HealthCheck{} + if err := copyViaJSON(dst, src); err != nil { + panic(err) + } + return dst +} + +func (hc *HealthCheck) DeepCopyObject() runtime.Object { + if c := DeepCopyTranslatorHealthCheck(hc); c != nil { + return c + } + return nil +}