Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit Events on THC (mis)configuration #2111

Merged
merged 2 commits into from
Jun 6, 2023
+304 −137
Merged

Emit Events on THC (mis)configuration #2111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f2b1ec4
Remove leftover TODO from PR#2096
DamianSawicki May 10, 2023
2e27a1b
Emit Events on THC (mis)configuration
DamianSawicki May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/annotations/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,8 @@ func (svc *Service) NEGAnnotation() (*NegAnnotation, bool, error) {
return &res, true, nil
}

// ShouldEnableTHC returns true if a THC annotation is found and its value is true.
func (svc *Service) ShouldEnableTHC() (bool, error) {
// IsThcAnnotated returns true if a THC annotation is found and its value is true.
func (svc *Service) IsThcAnnotated() (bool, error) {
var res THCAnnotation
annotation, ok := svc.v[THCAnnotationKey]
if !ok {
Expand Down
9 changes: 7 additions & 2 deletions pkg/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,14 +628,19 @@ func (lbc *LoadBalancerController) sync(key string) error {
}

// Bootstrap state for GCP sync.
urlMap, errs := lbc.Translator.TranslateIngress(ing, lbc.ctx.DefaultBackendSvcPort.ID, lbc.ctx.ClusterNamer)
urlMap, errs, warnings := lbc.Translator.TranslateIngress(ing, lbc.ctx.DefaultBackendSvcPort.ID, lbc.ctx.ClusterNamer)

if errs != nil {
msg := fmt.Errorf("invalid ingress spec: %v", utils.JoinErrs(errs))
lbc.ctx.Recorder(ing.Namespace).Eventf(ing, apiv1.EventTypeWarning, events.TranslateIngress, "Translation failed: %v", msg)
return msg
}

if warnings {
msg := "THC annotation is present for at least one Service, but the Transparent Health Checks feature is not enabled."
lbc.ctx.Recorder(ing.Namespace).Event(ing, apiv1.EventTypeWarning, "THCAnnotationWithoutFlag", msg)
}

// Sync GCP resources.
syncState := &syncState{urlMap, ing, nil}
syncErr := lbc.ingSyncer.Sync(syncState)
Expand Down Expand Up @@ -776,7 +781,7 @@ func updateAnnotations(client kubernetes.Interface, ing *v1.Ingress, newAnnotati
func (lbc *LoadBalancerController) ToSvcPorts(ings []*v1.Ingress) []utils.ServicePort {
var knownPorts []utils.ServicePort
for _, ing := range ings {
urlMap, _ := lbc.Translator.TranslateIngress(ing, lbc.ctx.DefaultBackendSvcPort.ID, lbc.ctx.ClusterNamer)
urlMap, _, _ := lbc.Translator.TranslateIngress(ing, lbc.ctx.DefaultBackendSvcPort.ID, lbc.ctx.ClusterNamer)
knownPorts = append(knownPorts, urlMap.AllServicePorts()...)
}
return knownPorts
Expand Down
94 changes: 58 additions & 36 deletions pkg/controller/translator/translator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,50 +206,67 @@ func (t *Translator) maybeEnableBackendConfig(sp *utils.ServicePort, svc *api_v1
return nil
}

// setEnableTHC sets the THCEnabled for the service port as true or false depending on whether
// setThcOptInOnSvc sets the THCOptInOnSvc for the service port as true or false depending on whether
// Transparent Health Checks should be enabled.
func (t *Translator) setEnableTHC(sp *utils.ServicePort, svc *api_v1.Service) {
THCEnabled := false
defer func() {
klog.Infof("Is THC enabled for the sevice %v with service port (%v, %v)? %v", svc.Name, sp.Port, sp.PortName, THCEnabled)
sp.THCEnabled = THCEnabled
}()
func (t *Translator) setThcOptInOnSvc(sp *utils.ServicePort, svc *api_v1.Service) (flagWarning bool) {
thcOptIn, err := annotations.FromService(svc).IsThcAnnotated()
if err != nil {
klog.Warningf("Parsing THC annotation failed: %+v.", err)
}
aojea marked this conversation as resolved.
Show resolved Hide resolved
if !thcOptIn {
sp.THCConfiguration.THCOptInOnSvc = false
klog.Infof("THCOptInOnSvc=%v for the sevice %v with service port (%v, %v).", sp.THCConfiguration.THCOptInOnSvc, svc.Name, sp.Port, sp.PortName)
return
}

// Feature flag for Transparent Health Checks not set.
if !t.enableTHC {
sp.THCConfiguration.THCEvents.THCAnnotationWithoutFlag = true
flagWarning = true

sp.THCConfiguration.THCOptInOnSvc = false
klog.Infof("THCOptInOnSvc=%v for the sevice %v with service port (%v, %v).", sp.THCConfiguration.THCOptInOnSvc, svc.Name, sp.Port, sp.PortName)
return
}

// There is a BackendConfig detailing the health check configuration for the service.
if sp.BackendConfig != nil && sp.BackendConfig.Spec.HealthCheck != nil {
sp.THCConfiguration.THCEvents.BackendConfigOverridesTHC = true

sp.THCConfiguration.THCOptInOnSvc = false
klog.Infof("THCOptInOnSvc=%v for the sevice %v with service port (%v, %v).", sp.THCConfiguration.THCOptInOnSvc, svc.Name, sp.Port, sp.PortName)
return
}

THCEnabled, err := annotations.FromService(svc).ShouldEnableTHC()
if err != nil {
message := fmt.Sprintf("Parsing THC annotation failed: %+v.", err)
t.recorderGetter.Recorder(sp.ID.Service.Namespace).Event(svc, api_v1.EventTypeWarning, "THCAnnotationParsingFailed", message)
klog.Warning(message)
}
// THC works only with NEGs (not Instance Groups) and this is not a Service with NEG enabled.
if !sp.NEGEnabled {
sp.THCConfiguration.THCEvents.THCAnnotationWithoutNEG = true

if THCEnabled && !sp.NEGEnabled {
message := "THC annotation present, but NEG is disabled. Will not enable Transparent Health Checks."
t.recorderGetter.Recorder(sp.ID.Service.Namespace).Event(svc, api_v1.EventTypeWarning, "THCAnnotationWithoutNEG", message)
klog.Warning(message)
THCEnabled = false
sp.THCConfiguration.THCOptInOnSvc = false
klog.Infof("THCOptInOnSvc=%v for the sevice %v with service port (%v, %v).", sp.THCConfiguration.THCOptInOnSvc, svc.Name, sp.Port, sp.PortName)
return
}

sp.THCConfiguration.THCEvents.THCConfigured = true

sp.THCConfiguration.THCOptInOnSvc = true
klog.Infof("THCOptInOnSvc=%v for the sevice %v with service port (%v, %v).", sp.THCConfiguration.THCOptInOnSvc, svc.Name, sp.Port, sp.PortName)
return
}

// getServicePort looks in the svc store for a matching service:port,
// and returns the nodeport.
func (t *Translator) getServicePort(id utils.ServicePortID, params *getServicePortParams, namer namer_util.BackendNamer) (*utils.ServicePort, error) {
// The returned bool indicates whether there is a warning being returned or not.
func (t *Translator) getServicePort(id utils.ServicePortID, params *getServicePortParams, namer namer_util.BackendNamer) (*utils.ServicePort, error, bool) {
svc, err := t.getCachedService(id)
if err != nil {
return nil, err
return nil, err, false
}

port := ServicePort(*svc, id.Port)
if port == nil {
// This is a fatal error.
return nil, errors.ErrSvcPortNotFound{ServicePortID: id}
return nil, errors.ErrSvcPortNotFound{ServicePortID: id}, false
}

// We periodically add information to the ServicePort to ensure that we
Expand All @@ -265,31 +282,33 @@ func (t *Translator) getServicePort(id utils.ServicePortID, params *getServicePo
}

if err := maybeEnableNEG(svcPort, svc); err != nil {
return nil, err
return nil, err, false
}

if err := setAppProtocol(svcPort, svc, port); err != nil {
return svcPort, err
return svcPort, err, false
}

if flags.F.EnableTrafficScaling {
if err := setTrafficScaling(svcPort, svc); err != nil {
return nil, err
return nil, err, false
}
}

if err := t.maybeEnableBackendConfig(svcPort, svc, port); err != nil {
return svcPort, err
return svcPort, err, false
}

t.setEnableTHC(svcPort, svc)
flagWarning := t.setThcOptInOnSvc(svcPort, svc)

return svcPort, nil
return svcPort, nil, flagWarning
}

// TranslateIngress converts an Ingress into our internal UrlMap representation.
func (t *Translator) TranslateIngress(ing *v1.Ingress, systemDefaultBackend utils.ServicePortID, namer namer_util.BackendNamer) (*utils.GCEURLMap, []error) {
// The returned bool is for warnings (there is one type of warnings currently possible).
func (t *Translator) TranslateIngress(ing *v1.Ingress, systemDefaultBackend utils.ServicePortID, namer namer_util.BackendNamer) (*utils.GCEURLMap, []error, bool) {
var errs []error
var warnings bool
urlMap := utils.NewGCEURLMap()

params := &getServicePortParams{}
Expand All @@ -308,7 +327,8 @@ func (t *Translator) TranslateIngress(ing *v1.Ingress, systemDefaultBackend util
errs = append(errs, err)
continue
}
svcPort, err := t.getServicePort(svcPortID, params, namer)
svcPort, err, warning := t.getServicePort(svcPortID, params, namer)
warnings = warnings || warning
if err != nil {
errs = append(errs, err)
}
Expand Down Expand Up @@ -342,26 +362,28 @@ func (t *Translator) TranslateIngress(ing *v1.Ingress, systemDefaultBackend util
svcPortID, err := utils.BackendToServicePortID(*ing.Spec.DefaultBackend, ing.Namespace)
if err != nil {
errs = append(errs, err)
return urlMap, errs
return urlMap, errs, warnings
}
svcPort, err := t.getServicePort(svcPortID, params, namer)
svcPort, err, warning := t.getServicePort(svcPortID, params, namer)
warnings = warnings || warning
if err == nil {
urlMap.DefaultBackend = svcPort
return urlMap, errs
return urlMap, errs, warnings
}

errs = append(errs, err)
return urlMap, errs
return urlMap, errs, warnings
}

svcPort, err := t.getServicePort(systemDefaultBackend, params, namer)
svcPort, err, warning := t.getServicePort(systemDefaultBackend, params, namer)
warnings = warnings || warning
if err == nil {
urlMap.DefaultBackend = svcPort
return urlMap, errs
return urlMap, errs, warnings
}

errs = append(errs, fmt.Errorf("failed to retrieve the system default backend service %q with port %q: %v", systemDefaultBackend.Service.String(), systemDefaultBackend.Port.String(), err))
return urlMap, errs
return urlMap, errs, warnings
}

// validateAndGetPaths will validate the path based on the specified path type and will return the
Expand Down
Loading