Skip to content

Commit

Permalink
JSONify healthcheck Description for BackendConfig
Browse files Browse the repository at this point in the history
  • Loading branch information
DamianSawicki committed Apr 7, 2023
1 parent 4c7c629 commit 9d454d8
Show file tree
Hide file tree
Showing 10 changed files with 281 additions and 37 deletions.
7 changes: 1 addition & 6 deletions cmd/glbc/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,7 @@ func runControllers(ctx *ingctx.ControllerContext) {
// In NonGCP mode, use the zone specified in gce.conf directly.
// This overrides the zone/fault-domain label on nodes for NEG controller.
if flags.F.EnableNonGCPMode {
zone, err := ctx.Cloud.GetZone(context.Background())
if err != nil {
klog.Errorf("Failed to retrieve zone information from Cloud provider: %v; Please check if local-zone is specified in gce.conf.", err)
} else {
zoneGetter = negtypes.NewSimpleZoneGetter(zone.FailureDomain)
}
zoneGetter = negtypes.NewSimpleZoneGetter(ctx.Cloud.Zone())
}

enableAsm := false
Expand Down
34 changes: 33 additions & 1 deletion pkg/healthchecks/healthchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/ingress-gce/pkg/loadbalancers/features"
"k8s.io/ingress-gce/pkg/translator"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/descutils"
"k8s.io/klog/v2"
)

Expand All @@ -43,13 +44,30 @@ 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
clusterInfo descutils.ClusterInfo
}

// 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}
ci := generateClusterInfo(cloud.(*gce.Cloud))
return &HealthChecks{cloud, healthCheckPath, defaultBackendSvc, ci}
}

func generateClusterInfo(gceCloud *gce.Cloud) descutils.ClusterInfo {
var location string
regionalCluster := gceCloud.Regional()
if regionalCluster {
location = gceCloud.Region()
} else {
location = gceCloud.Zone()
}
name, err := gceCloud.ClusterID.GetID()
if err != nil {
klog.Errorf("Failed to obtain cluster name, %+v", err)
}
return descutils.ClusterInfo{Name: name, Location: location, Regional: regionalCluster}
}

// new returns a *HealthCheck with default settings and specified port/protocol
Expand All @@ -67,9 +85,23 @@ func (h *HealthChecks) new(sp utils.ServicePort) *translator.HealthCheck {
hc.Name = sp.BackendName()
hc.Port = sp.NodePort
hc.RequestPath = h.pathFromSvcPort(sp)
hc.SetHealthcheckInfo(h.generateHealthcheckInfo(hc))
return hc
}

func (h *HealthChecks) generateHealthcheckInfo(hc *translator.HealthCheck) descutils.HealthcheckInfo {
ingressType := descutils.ExternalLB
if hc.ForILB {
ingressType = descutils.InternalLB
}
return descutils.HealthcheckInfo{
ClusterInfo: h.clusterInfo,
ServiceInfo: descutils.ServiceInfo(h.defaultBackendSvc),
HealthcheckConfig: descutils.DefaultHC,
IngressType: ingressType,
}
}

// SyncServicePort implements HealthChecker.
func (h *HealthChecks) SyncServicePort(sp *utils.ServicePort, probe *v1.Probe) (string, error) {
hc := h.new(*sp)
Expand Down
90 changes: 72 additions & 18 deletions pkg/healthchecks/healthchecks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,9 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) {
flags.F.EnableUpdateCustomHealthCheckDescription = oldUpdateDescription
}()

fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
testClusterValues := gce.DefaultTestClusterValues()
testClusterValues.Regional = true
fakeGCE := gce.NewFakeGCECloud(testClusterValues)

var (
defaultSP *utils.ServicePort = testSPs["HTTP-80-reg-nil"]
Expand Down Expand Up @@ -503,9 +505,17 @@ func TestRolloutUpdateCustomHCDescription(t *testing.T) {

outputBCHCWithFlag := getSingletonHealthcheck(t, fakeGCE)

if outputBCHCWithFlag.Description != translator.DescriptionForHealthChecksFromBackendConfig {
wantDescription := "" +
fmt.Sprintf("{\n") +
fmt.Sprintf(" \"K8sCluster\": \"/locations/%s/clusters/%s\",\n", gce.DefaultTestClusterValues().Region, gce.DefaultTestClusterValues().ClusterID) +
fmt.Sprintf(" \"K8sResource\": \"/namespaces/%s/services/%s\",\n", defaultBackendSvc.Namespace, defaultBackendSvc.Name) +
fmt.Sprintf(" \"K8sResourceDependency\": \"Ingress External Load Balancer\",\n") +
fmt.Sprintf(" \"Config\": \"BackendConfig\"\n") +
fmt.Sprintf("}")

if outputBCHCWithFlag.Description != wantDescription {
t.Fatalf("incorrect Description, is: \"%v\", want: \"%v\"",
outputBCHCWithFlag.Description, translator.DescriptionForHealthChecksFromBackendConfig)
outputBCHCWithFlag.Description, wantDescription)
}

// Verify that only the Description is modified after rollout.
Expand Down Expand Up @@ -980,11 +990,12 @@ func TestSyncServicePort(t *testing.T) {
}()

type tc struct {
desc string
setup func(*cloud.MockGCE)
sp *utils.ServicePort
probe *v1.Probe
regional bool
desc string
setup func(*cloud.MockGCE)
sp *utils.ServicePort
probe *v1.Probe
regional bool
regionalCluster bool

wantSelfLink string
wantErr bool
Expand Down Expand Up @@ -1066,7 +1077,10 @@ func TestSyncServicePort(t *testing.T) {
chc = fixture.hc()
chc.HttpHealthCheck.RequestPath = "/foo"
chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
cases = append(cases, &tc{desc: "create backendconfig", sp: testSPs["HTTP-80-reg-bc"], wantComputeHC: chc})
cases = append(cases, &tc{
desc: "create backendconfig",
sp: testSPs["HTTP-80-reg-bc"],
wantComputeHC: chc})

// BackendConfig all
chc = fixture.hc()
Expand All @@ -1079,7 +1093,10 @@ func TestSyncServicePort(t *testing.T) {
// PortSpecification is set by the controller
chc.HttpHealthCheck.PortSpecification = "USE_FIXED_PORT"
chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
cases = append(cases, &tc{desc: "create backendconfig all", sp: testSPs["HTTP-80-reg-bcall"], wantComputeHC: chc})
cases = append(cases, &tc{
desc: "create backendconfig all",
sp: testSPs["HTTP-80-reg-bcall"],
wantComputeHC: chc})

i64 := func(i int64) *int64 { return &i }

Expand Down Expand Up @@ -1118,6 +1135,18 @@ func TestSyncServicePort(t *testing.T) {
wantComputeHC: chc,
})

// BackendConfig ilb
chc = fixture.ilb()
chc.HttpHealthCheck.RequestPath = "/foo"
chc.Description = translator.DescriptionForHealthChecksFromBackendConfig
cases = append(cases, &tc{
desc: "create backendconfig ilb regional cluster",
sp: testSPs["HTTP-80-ilb-bc"],
regional: true,
regionalCluster: true,
wantComputeHC: chc,
})

// Probe and BackendConfig override
chc = fixture.hc()
chc.HttpHealthCheck.RequestPath = "/foo"
Expand Down Expand Up @@ -1325,15 +1354,40 @@ func TestSyncServicePort(t *testing.T) {
// settings.

for _, tc := range cases {
tc := *tc
tc.updateHCDescription = true
tc.desc = tc.desc + " with updateHCDescription"
cases = append(cases, tc)
copyOfWant := *tc.wantComputeHC
if tc.sp.BackendConfig != nil {
var wantLocation, wantIngressType string
if tc.regionalCluster {
wantLocation = gce.DefaultTestClusterValues().Region
} else {
wantLocation = gce.DefaultTestClusterValues().ZoneName
}
if tc.sp.L7ILBEnabled {
wantIngressType = "Ingress Internal Load Balancer"
} else {
wantIngressType = "Ingress External Load Balancer"
}
copyOfWant.Description = "" +
fmt.Sprintf("{\n") +
fmt.Sprintf(" \"K8sCluster\": \"/locations/%s/clusters/%s\",\n", wantLocation, gce.DefaultTestClusterValues().ClusterID) +
fmt.Sprintf(" \"K8sResource\": \"/namespaces/%s/services/%s\",\n", defaultBackendSvc.Namespace, defaultBackendSvc.Name) +
fmt.Sprintf(" \"K8sResourceDependency\": \"%s\",\n", wantIngressType) +
fmt.Sprintf(" \"Config\": \"BackendConfig\"\n") +
fmt.Sprintf("}")
}
tc.wantComputeHC = &copyOfWant
cases = append(cases, &tc)
}

for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
flags.F.EnableUpdateCustomHealthCheckDescription = tc.updateHCDescription
fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues())
testClusterValues := gce.DefaultTestClusterValues()
testClusterValues.Regional = tc.regionalCluster
fakeGCE := gce.NewFakeGCECloud(testClusterValues)

mock := fakeGCE.Compute().(*cloud.MockGCE)
setupMockUpdate(mock)
Expand Down Expand Up @@ -1364,20 +1418,20 @@ func TestSyncServicePort(t *testing.T) {
t.Fatalf("Got %d healthchecks, want 1\n%s", len(computeHCs), pretty.Sprint(computeHCs))
}

gotHC := computeHCs[0]
// Filter out SelfLink because it is hard to deal with in the mock and
// test cases.
filter := func(hc *compute.HealthCheck) {
filter := func(hc compute.HealthCheck) compute.HealthCheck {
hc.SelfLink = ""
if !tc.updateHCDescription {
hc.Description = ""
}
return hc
}
filter(gotHC)
filter(tc.wantComputeHC)
gotHC := filter(*computeHCs[0])
wantHC := filter(*tc.wantComputeHC)

if !reflect.DeepEqual(gotHC, tc.wantComputeHC) {
t.Fatalf("Compute healthcheck is:\n%s, want:\n%s", pretty.Sprint(gotHC), pretty.Sprint(tc.wantComputeHC))
if !reflect.DeepEqual(gotHC, wantHC) {
t.Fatalf("Compute healthcheck is:\n%s, want:\n%s", pretty.Sprint(gotHC), pretty.Sprint(wantHC))
}
}

Expand Down
6 changes: 1 addition & 5 deletions pkg/psc/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,7 @@ func NewController(ctx *context.ControllerContext) *Controller {
if controller.regionalCluster {
controller.clusterLoc = controller.cloud.Region()
} else {
zone, err := controller.cloud.GetZone(context2.Background())
if err != nil {
klog.Errorf("Failed to retrieve zone information from cloud provider: %q", err)
}
controller.clusterLoc = zone.FailureDomain
controller.clusterLoc = controller.cloud.Zone()
}

ctx.SAInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
Expand Down
7 changes: 2 additions & 5 deletions pkg/psc/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,11 +301,8 @@ func TestServiceAttachmentCreation(t *testing.T) {
t.Errorf("%s", err)
}

zone, err := fakeCloud.GetZone(context2.TODO())
if err != nil {
t.Errorf("failed to get zone %q", err)
}
desc := sautils.NewServiceAttachmentDesc(testNamespace, saName, ClusterName, zone.FailureDomain, false)
zone := fakeCloud.Zone()
desc := sautils.NewServiceAttachmentDesc(testNamespace, saName, ClusterName, zone, false)

expectedSA := &ga.ServiceAttachment{
ConnectionPreference: tc.connectionPreference,
Expand Down
17 changes: 17 additions & 0 deletions pkg/translator/healthchecks.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/ingress-gce/pkg/flags"
"k8s.io/ingress-gce/pkg/loadbalancers/features"
"k8s.io/ingress-gce/pkg/utils"
"k8s.io/ingress-gce/pkg/utils/descutils"
"k8s.io/klog/v2"
)

Expand Down Expand Up @@ -90,6 +91,18 @@ type HealthCheck struct {
// compute struct back.
computealpha.HTTPHealthCheck
computealpha.HealthCheck
healthcheckInfo descutils.HealthcheckInfo
}

func (hc *HealthCheck) SetHealthcheckInfo(i descutils.HealthcheckInfo) {
hc.healthcheckInfo = i
hc.reconcileHCDescription()
}

func (hc *HealthCheck) reconcileHCDescription() {
if flags.F.EnableUpdateCustomHealthCheckDescription && hc.healthcheckInfo.HealthcheckConfig == descutils.BackendConfigHC {
hc.Description = hc.healthcheckInfo.GenerateHealthcheckDescription()
}
}

// NewHealthCheck creates a HealthCheck which abstracts nested structs away
Expand Down Expand Up @@ -227,6 +240,8 @@ func (hc *HealthCheck) UpdateFromBackendConfig(c *backendconfigv1.HealthCheckCon
if flags.F.EnableUpdateCustomHealthCheckDescription {
hc.Description = DescriptionForHealthChecksFromBackendConfig
}
hc.healthcheckInfo.HealthcheckConfig = descutils.BackendConfigHC
hc.reconcileHCDescription()
}

// DefaultHealthCheck simply returns the default health check.
Expand Down Expand Up @@ -337,4 +352,6 @@ func ApplyProbeSettingsToHC(p *v1.Probe, hc *HealthCheck) {
}

hc.Description = DescriptionForHealthChecksFromReadinessProbe
hc.healthcheckInfo.HealthcheckConfig = descutils.ReadinessProbeHC
hc.reconcileHCDescription()
}
72 changes: 71 additions & 1 deletion pkg/utils/descutils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,64 @@ limitations under the License.

package descutils

import "fmt"
import (
"encoding/json"
"fmt"

"k8s.io/apimachinery/pkg/types"
"k8s.io/klog/v2"
)

type ClusterInfo struct {
Name string
Location string
Regional bool
}

type ServiceInfo types.NamespacedName

type HealthcheckConfig string

const (
DefaultHC HealthcheckConfig = "Default"
ReadinessProbeHC HealthcheckConfig = "ReadinessProbe"
BackendConfigHC HealthcheckConfig = "BackendConfig"
)

type IngressType string

const (
InternalLB IngressType = "Ingress Internal Load Balancer"
ExternalLB IngressType = "Ingress External Load Balancer"
)

type HealthcheckInfo struct {
ClusterInfo
ServiceInfo
HealthcheckConfig
IngressType
}

type HealthcheckDesc struct {
K8sCluster string
K8sResource string
K8sResourceDependency IngressType
Config HealthcheckConfig
}

func (i *ClusterInfo) generateClusterDescription() string {
// locType here differs from locType in GenerateClusterLink().
locType := "locations"
return fmt.Sprintf("/%s/%s/clusters/%s", locType, i.Location, i.Name)
}

func NewServiceInfo(namespace, resourceName string) ServiceInfo {
return ServiceInfo{namespace, resourceName}
}

func (i *ServiceInfo) generateK8sResourceDescription() string {
return GenerateK8sResourceLink(i.Namespace, "services", i.Name)
}

func GenerateClusterLink(name, location string, regional bool) string {
if name == "" || location == "" {
Expand All @@ -33,3 +90,16 @@ func GenerateClusterLink(name, location string, regional bool) string {
func GenerateK8sResourceLink(namespace, resourceType, resourceName string) string {
return fmt.Sprintf("/namespaces/%s/%s/%s", namespace, resourceType, resourceName)
}

func (i *HealthcheckInfo) GenerateHealthcheckDescription() string {
desc := HealthcheckDesc{}
desc.K8sCluster = i.ClusterInfo.generateClusterDescription()
desc.K8sResource = i.ServiceInfo.generateK8sResourceDescription()
desc.K8sResourceDependency = i.IngressType
desc.Config = i.HealthcheckConfig
json, err := json.MarshalIndent(desc, "", " ")
if err != nil {
klog.Error("Failed to marshall HealthcheckDesc %s: %v", desc, err)
}
return string(json)
}
Loading

0 comments on commit 9d454d8

Please sign in to comment.