Skip to content

Commit

Permalink
Emit event on Description-only healthcheck update
Browse files Browse the repository at this point in the history
  • Loading branch information
DamianSawicki committed Apr 12, 2023
1 parent 4c7c629 commit 9d5fea2
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 13 deletions.
3 changes: 2 additions & 1 deletion pkg/backends/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion pkg/backends/syncer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand Down
4 changes: 3 additions & 1 deletion pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
8 changes: 8 additions & 0 deletions pkg/healthchecks/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)) }
Expand Down
11 changes: 9 additions & 2 deletions pkg/healthchecks/healthchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
30 changes: 22 additions & 8 deletions pkg/healthchecks/healthchecks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
32 changes: 32 additions & 0 deletions pkg/translator/healthchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package translator

import (
"encoding/json"
"errors"
"fmt"
"strings"
Expand All @@ -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"
Expand Down Expand Up @@ -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
}

0 comments on commit 9d5fea2

Please sign in to comment.