Skip to content

Commit

Permalink
Migrated pkg/proxy to structured logging (kubernetes#104891)
Browse files Browse the repository at this point in the history 
* migrated service.go to structured logging

* fixing capital letter in starting

* migrated topology.go

* migrated endpointslicecache.go

* migrated endpoints.go

* nit typo

* nit plural to singular

* fixed format

* code formatting

* resolving review comment for key ipFamily

* resolving review comment for key endpoints.go

* code formating

* Converted Warningf to ErrorS, wherever applicable

* included review changes

* included review changes
  • Loading branch information
@shivanshuraj1333 @tallclair
shivanshuraj1333 authored and tallclair committed Oct 19, 2021
1 parent bd99701 commit 113b663
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 29 deletions.
20 changes: 10 additions & 10 deletions pkg/proxy/endpoints.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func (ect *EndpointChangeTracker) Update(previous, current *v1.Endpoints) bool {
delete(ect.lastChangeTriggerTimes, namespacedName)
} else {
for spn, eps := range change.current {
klog.V(2).Infof("Service port %s updated: %d endpoints", spn, len(eps))
klog.V(2).InfoS("Service port endpoints update", "servicePort", spn, "endpoints", len(eps))
}
}

Expand All @@ -259,19 +259,19 @@ func (ect *EndpointChangeTracker) Update(previous, current *v1.Endpoints) bool {
// If removeSlice is true, slice will be removed, otherwise it will be added or updated.
func (ect *EndpointChangeTracker) EndpointSliceUpdate(endpointSlice *discovery.EndpointSlice, removeSlice bool) bool {
if !supportedEndpointSliceAddressTypes.Has(string(endpointSlice.AddressType)) {
klog.V(4).Infof("EndpointSlice address type not supported by kube-proxy: %s", endpointSlice.AddressType)
klog.V(4).InfoS("EndpointSlice address type not supported by kube-proxy", "addressType", endpointSlice.AddressType)
return false
}

// This should never happen
if endpointSlice == nil {
klog.Error("Nil endpointSlice passed to EndpointSliceUpdate")
klog.ErrorS(nil, "Nil endpointSlice passed to EndpointSliceUpdate")
return false
}

namespacedName, _, err := endpointSliceCacheKeys(endpointSlice)
if err != nil {
klog.Warningf("Error getting endpoint slice cache keys: %v", err)
klog.InfoS("Error getting endpoint slice cache keys", "err", err)
return false
}

Expand Down Expand Up @@ -349,8 +349,8 @@ func getLastChangeTriggerTime(annotations map[string]string) time.Time {
}
val, err := time.Parse(time.RFC3339Nano, annotations[v1.EndpointsLastChangeTriggerTime])
if err != nil {
klog.Warningf("Error while parsing EndpointsLastChangeTriggerTimeAnnotation: '%s'. Error is %v",
annotations[v1.EndpointsLastChangeTriggerTime], err)
klog.ErrorS(err, "Error while parsing EndpointsLastChangeTriggerTimeAnnotation",
"value", annotations[v1.EndpointsLastChangeTriggerTime])
// In case of error val = time.Zero, which is ignored in the upstream code.
}
return val
Expand Down Expand Up @@ -419,7 +419,7 @@ func (ect *EndpointChangeTracker) endpointsToEndpointsMap(endpoints *v1.Endpoint
for i := range ss.Ports {
port := &ss.Ports[i]
if port.Port == 0 {
klog.Warningf("ignoring invalid endpoint port %s", port.Name)
klog.ErrorS(nil, "Ignoring invalid endpoint port", "portName", port.Name)
continue
}
svcPortName := ServicePortName{
Expand All @@ -430,7 +430,7 @@ func (ect *EndpointChangeTracker) endpointsToEndpointsMap(endpoints *v1.Endpoint
for i := range ss.Addresses {
addr := &ss.Addresses[i]
if addr.IP == "" {
klog.Warningf("ignoring invalid endpoint port %s with empty host", port.Name)
klog.ErrorS(nil, "Ignoring invalid endpoint port with empty host", "portName", port.Name)
continue
}

Expand Down Expand Up @@ -466,7 +466,7 @@ func (ect *EndpointChangeTracker) endpointsToEndpointsMap(endpoints *v1.Endpoint
}
}

klog.V(3).Infof("Setting endpoints for %q to %+v", svcPortName, formatEndpointsList(endpointsMap[svcPortName]))
klog.V(3).InfoS("Setting endpoints for service port", "portName", svcPortName, "endpoints", formatEndpointsList(endpointsMap[svcPortName]))
}
}
return endpointsMap
Expand Down Expand Up @@ -550,7 +550,7 @@ func detectStaleConnections(oldEndpointsMap, newEndpointsMap EndpointsMap, stale
}
}
if stale {
klog.V(4).Infof("Stale endpoint %v -> %v", svcPortName, ep.String())
klog.V(4).InfoS("Stale endpoint", "portName", svcPortName, "endpoint", ep)
*staleEndpoints = append(*staleEndpoints, ServiceEndpoint{Endpoint: ep.String(), ServicePortName: svcPortName})
}
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/proxy/endpointslicecache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func standardEndpointInfo(ep *BaseEndpointInfo) Endpoint {
func (cache *EndpointSliceCache) updatePending(endpointSlice *discovery.EndpointSlice, remove bool) bool {
serviceKey, sliceKey, err := endpointSliceCacheKeys(endpointSlice)
if err != nil {
klog.Warningf("Error getting endpoint slice cache keys: %v", err)
klog.ErrorS(err, "Error getting endpoint slice cache keys")
return false
}

Expand Down Expand Up @@ -235,12 +235,12 @@ func (cache *EndpointSliceCache) endpointInfoByServicePort(serviceNN types.Names
for _, sliceInfo := range sliceInfoByName {
for _, port := range sliceInfo.Ports {
if port.Name == nil {
klog.Warningf("ignoring port with nil name %v", port)
klog.ErrorS(nil, "Ignoring port with nil name", "portName", port.Name)
continue
}
// TODO: handle nil ports to mean "all"
if port.Port == nil || *port.Port == int32(0) {
klog.Warningf("ignoring invalid endpoint port %s", *port.Name)
klog.ErrorS(nil, "Ignoring invalid endpoint port", "portName", *port.Name)
continue
}

Expand All @@ -266,7 +266,7 @@ func (cache *EndpointSliceCache) addEndpoints(serviceNN types.NamespacedName, po
// iterate through endpoints to add them to endpointSet.
for _, endpoint := range endpoints {
if len(endpoint.Addresses) == 0 {
klog.Warningf("ignoring invalid endpoint port %s with empty addresses", endpoint)
klog.ErrorS(nil, "Ignoring invalid endpoint port with empty address", "endpoint", endpoint)
continue
}

Expand Down Expand Up @@ -355,7 +355,7 @@ func endpointsMapFromEndpointInfo(endpointInfoBySP map[ServicePortName]map[strin
// Ensure endpoints are always returned in the same order to simplify diffing.
sort.Sort(byEndpoint(endpointsMap[svcPortName]))

klog.V(3).Infof("Setting endpoints for %q to %+v", svcPortName, formatEndpointsList(endpointsMap[svcPortName]))
klog.V(3).InfoS("Setting endpoints for service port name", "portName", svcPortName, "endpoints", formatEndpointsList(endpointsMap[svcPortName]))
}
}

Expand Down
22 changes: 12 additions & 10 deletions pkg/proxy/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,16 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic

// Log the IPs not matching the ipFamily
if ips, ok := ipFamilyMap[utilproxy.OtherIPFamily(sct.ipFamily)]; ok && len(ips) > 0 {
klog.V(4).Infof("service change tracker(%v) ignored the following external IPs(%s) for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(ips, ","), service.Namespace, service.Name)
klog.V(4).InfoS("Service change tracker ignored the following external IPs for given service as they don't match IP Family",
"ipFamily", sct.ipFamily, "externalIPs", strings.Join(ips, ","), "service", klog.KObj(service))
}

ipFamilyMap = utilproxy.MapCIDRsByIPFamily(loadBalancerSourceRanges)
info.loadBalancerSourceRanges = ipFamilyMap[sct.ipFamily]
// Log the CIDRs not matching the ipFamily
if cidrs, ok := ipFamilyMap[utilproxy.OtherIPFamily(sct.ipFamily)]; ok && len(cidrs) > 0 {
klog.V(4).Infof("service change tracker(%v) ignored the following load balancer source ranges(%s) for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(cidrs, ","), service.Namespace, service.Name)
klog.V(4).InfoS("Service change tracker ignored the following load balancer source ranges for given Service as they don't match IP Family",
"ipFamily", sct.ipFamily, "loadBalancerSourceRanges", strings.Join(cidrs, ","), "service", klog.KObj(service))
}

// Obtain Load Balancer Ingress IPs
Expand All @@ -204,8 +206,8 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
ipFamilyMap = utilproxy.MapIPsByIPFamily(ips)

if ipList, ok := ipFamilyMap[utilproxy.OtherIPFamily(sct.ipFamily)]; ok && len(ipList) > 0 {
klog.V(4).Infof("service change tracker(%v) ignored the following load balancer(%s) ingress ips for service %v/%v as they don't match IPFamily", sct.ipFamily, strings.Join(ipList, ","), service.Namespace, service.Name)

klog.V(4).InfoS("Service change tracker ignored the following load balancer ingress IPs for given Service as they don't match the IP Family",
"ipFamily", sct.ipFamily, "loadBalancerIngressIps", strings.Join(ipList, ","), "service", klog.KObj(service))
}
// Create the LoadBalancerStatus with the filtered IPs
for _, ip := range ipFamilyMap[sct.ipFamily] {
Expand All @@ -216,7 +218,7 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
if apiservice.NeedsHealthCheck(service) {
p := service.Spec.HealthCheckNodePort
if p == 0 {
klog.Errorf("Service %s/%s has no healthcheck nodeport", service.Namespace, service.Name)
klog.ErrorS(nil, "Service has no healthcheck nodeport", "service", klog.KObj(service))
} else {
info.healthCheckNodePort = int(p)
}
Expand Down Expand Up @@ -299,7 +301,7 @@ func (sct *ServiceChangeTracker) Update(previous, current *v1.Service) bool {
if reflect.DeepEqual(change.previous, change.current) {
delete(sct.items, namespacedName)
} else {
klog.V(2).Infof("Service %s updated: %d ports", namespacedName, len(change.current))
klog.V(2).InfoS("Service updated ports", "service", klog.KObj(svc), "portCount", len(change.current))
}
metrics.ServiceChangesPending.Set(float64(len(sct.items)))
return len(sct.items) > 0
Expand Down Expand Up @@ -414,9 +416,9 @@ func (sm *ServiceMap) merge(other ServiceMap) sets.String {
existingPorts.Insert(svcPortName.String())
_, exists := (*sm)[svcPortName]
if !exists {
klog.V(1).Infof("Adding new service port %q at %s", svcPortName, info.String())
klog.V(1).InfoS("Adding new service port", "portName", svcPortName, "servicePort", info)
} else {
klog.V(1).Infof("Updating existing service port %q at %s", svcPortName, info.String())
klog.V(1).InfoS("Updating existing service port", "portName", svcPortName, "servicePort", info)
}
(*sm)[svcPortName] = info
}
Expand All @@ -439,13 +441,13 @@ func (sm *ServiceMap) unmerge(other ServiceMap, UDPStaleClusterIP sets.String) {
for svcPortName := range other {
info, exists := (*sm)[svcPortName]
if exists {
klog.V(1).Infof("Removing service port %q", svcPortName)
klog.V(1).InfoS("Removing service port", "portName", svcPortName)
if info.Protocol() == v1.ProtocolUDP {
UDPStaleClusterIP.Insert(info.ClusterIP().String())
}
delete(*sm, svcPortName)
} else {
klog.Errorf("Service port %q doesn't exists", svcPortName)
klog.ErrorS(nil, "Service port does not exists", "portName", svcPortName)
}
}
}
8 changes: 4 additions & 4 deletions pkg/proxy/topology.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,22 @@ func FilterEndpoints(endpoints []Endpoint, svcInfo ServicePort, nodeLabels map[s
func filterEndpointsWithHints(endpoints []Endpoint, hintsAnnotation string, nodeLabels map[string]string) []Endpoint {
if hintsAnnotation != "Auto" && hintsAnnotation != "auto" {
if hintsAnnotation != "" && hintsAnnotation != "Disabled" && hintsAnnotation != "disabled" {
klog.Warningf("Skipping topology aware endpoint filtering since Service has unexpected value for %s annotation: %s", v1.AnnotationTopologyAwareHints, hintsAnnotation)
klog.InfoS("Skipping topology aware endpoint filtering since Service has unexpected value", "annotationTopologyAwareHints", v1.AnnotationTopologyAwareHints, "hints", hintsAnnotation)
}
return endpoints
}

zone, ok := nodeLabels[v1.LabelTopologyZone]
if !ok || zone == "" {
klog.Warningf("Skipping topology aware endpoint filtering since node is missing %s label", v1.LabelTopologyZone)
klog.InfoS("Skipping topology aware endpoint filtering since node is missing label", "label", v1.LabelTopologyZone)
return endpoints
}

filteredEndpoints := []Endpoint{}

for _, endpoint := range endpoints {
if endpoint.GetZoneHints().Len() == 0 {
klog.Warningf("Skipping topology aware endpoint filtering since one or more endpoints is missing a zone hint")
klog.InfoS("Skipping topology aware endpoint filtering since one or more endpoints is missing a zone hint")
return endpoints
}
if endpoint.GetZoneHints().Has(zone) {
Expand All @@ -78,7 +78,7 @@ func filterEndpointsWithHints(endpoints []Endpoint, hintsAnnotation string, node
}

if len(filteredEndpoints) == 0 {
klog.Warningf("Skipping topology aware endpoint filtering since no hints were provided for zone %s", zone)
klog.InfoS("Skipping topology aware endpoint filtering since no hints were provided for zone", "zone", zone)
return endpoints
}

Expand Down

0 comments on commit 113b663

Please sign in to comment.