From f01e3e931b93351b7d569c5dece07e43b64e19ca Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Tue, 22 Jun 2021 16:19:03 -0700 Subject: [PATCH 1/5] Get rid of redundant logs in HCN version range checks Kubeproxy logs are filled with redudnant version check spam from an unexported call that's invoked as part of checking if a feature is supported. The logs don't detail what feature(s) are even being checked so it just seems like spam. With the way things are implemented all of the hcn features are checked for support in any of the `hcn.XSupported()` calls not just the one being checked, so these logs come up quite a bit if there's many features that aren't supported on the machine. Add two new logs in a sync.Once that logs the HNS version and supported features. This should be enough to investigate version issues. Should remedy https://github.com/microsoft/hcsshim/issues/1043 Signed-off-by: Daniel Canter (cherry picked from commit 6288bb97125c8a25d79b87ffd3a597258fa4e0bc) Signed-off-by: Daniel Canter --- hcn/hcnsupport.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/hcn/hcnsupport.go b/hcn/hcnsupport.go index 1d9fe762e9..f15a5d34ee 100644 --- a/hcn/hcnsupport.go +++ b/hcn/hcnsupport.go @@ -1,9 +1,14 @@ package hcn import ( + "fmt" + "sync" + "github.com/sirupsen/logrus" ) +var versionOnce sync.Once + // SupportedFeatures are the features provided by the Service. type SupportedFeatures struct { Acl AclFeatures `json:"ACL"` @@ -72,6 +77,17 @@ func GetSupportedFeatures() SupportedFeatures { features.L4WfpProxy = isFeatureSupported(globals.Version, L4WfpProxyPolicyVersion) features.TierAcl = isFeatureSupported(globals.Version, TierAclPolicyVersion) + // Only print the HCN version and features supported once, instead of everytime this is invoked. These logs are useful to + // debug incidents where there's confusion on if a feature is supported on the host machine. The sync.Once helps to avoid redundant + // spam of these anytime a check needs to be made for if an HCN feature is supported. This is a common occurrence in kubeproxy + // for example. + versionOnce.Do(func() { + logrus.WithFields(logrus.Fields{ + "version": fmt.Sprintf("%+v", globals.Version), + "supportedFeatures": fmt.Sprintf("%+v", features), + }).Info("HCN feature check") + }) + return features } @@ -87,19 +103,15 @@ func isFeatureSupported(currentVersion Version, versionsSupported VersionRanges) func isFeatureInRange(currentVersion Version, versionRange VersionRange) bool { if currentVersion.Major < versionRange.MinVersion.Major { - logrus.Infof("currentVersion.Major < versionRange.MinVersion.Major: %v, %v", currentVersion.Major, versionRange.MinVersion.Major) return false } if currentVersion.Major > versionRange.MaxVersion.Major { - logrus.Infof("currentVersion.Major > versionRange.MaxVersion.Major: %v, %v", currentVersion.Major, versionRange.MaxVersion.Major) return false } if currentVersion.Major == versionRange.MinVersion.Major && currentVersion.Minor < versionRange.MinVersion.Minor { - logrus.Infof("currentVersion.Minor < versionRange.MinVersion.Major: %v, %v", currentVersion.Minor, versionRange.MinVersion.Minor) return false } if currentVersion.Major == versionRange.MaxVersion.Major && currentVersion.Minor > versionRange.MaxVersion.Minor { - logrus.Infof("currentVersion.Minor > versionRange.MaxVersion.Major: %v, %v", currentVersion.Minor, versionRange.MaxVersion.Minor) return false } return true From e8ef1bbe09125c5612c79c2a63d83461def0c9ee Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Wed, 18 Aug 2021 20:06:20 -0700 Subject: [PATCH 2/5] Add GetCachedSupportedFeatures method to hcn package To avoid a breaking change on GetSupportedFeatures introduce a new GetCachedSupportedFeatures method. This method does the feature check and version parsing once and then assigns a global with the information. This can be used to optimize for situations where many uses of the hcn.IsXSupported methods are going to be used (kube-proxy for example). Signed-off-by: Daniel Canter (cherry picked from commit 112b5e7a30d0cac219085a26fc3a8188bc471db9) Signed-off-by: Daniel Canter --- hcn/hcn.go | 65 +++++++++++++++++++++++++++++++-------- hcn/hcnsupport.go | 77 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 117 insertions(+), 25 deletions(-) diff --git a/hcn/hcn.go b/hcn/hcn.go index 82fddee9f7..eefd88d856 100644 --- a/hcn/hcn.go +++ b/hcn/hcn.go @@ -127,7 +127,10 @@ func platformDoesNotSupportError(featureName string) error { // V2ApiSupported returns an error if the HCN version does not support the V2 Apis. func V2ApiSupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.Api.V2 { return nil } @@ -143,7 +146,10 @@ func V2SchemaVersion() SchemaVersion { // RemoteSubnetSupported returns an error if the HCN version does not support Remote Subnet policies. func RemoteSubnetSupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.RemoteSubnet { return nil } @@ -152,7 +158,10 @@ func RemoteSubnetSupported() error { // HostRouteSupported returns an error if the HCN version does not support Host Route policies. func HostRouteSupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.HostRoute { return nil } @@ -161,7 +170,10 @@ func HostRouteSupported() error { // DSRSupported returns an error if the HCN version does not support Direct Server Return. func DSRSupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.DSR { return nil } @@ -170,7 +182,10 @@ func DSRSupported() error { // Slash32EndpointPrefixesSupported returns an error if the HCN version does not support configuring endpoints with /32 prefixes. func Slash32EndpointPrefixesSupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.Slash32EndpointPrefixes { return nil } @@ -179,7 +194,10 @@ func Slash32EndpointPrefixesSupported() error { // AclSupportForProtocol252Supported returns an error if the HCN version does not support HNS ACL Policies to support protocol 252 for VXLAN. func AclSupportForProtocol252Supported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.AclSupportForProtocol252 { return nil } @@ -188,7 +206,10 @@ func AclSupportForProtocol252Supported() error { // SessionAffinitySupported returns an error if the HCN version does not support Session Affinity. func SessionAffinitySupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.SessionAffinity { return nil } @@ -197,7 +218,10 @@ func SessionAffinitySupported() error { // IPv6DualStackSupported returns an error if the HCN version does not support IPv6DualStack. func IPv6DualStackSupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.IPv6DualStack { return nil } @@ -206,7 +230,10 @@ func IPv6DualStackSupported() error { //L4proxySupported returns an error if the HCN verison does not support L4Proxy func L4proxyPolicySupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.L4Proxy { return nil } @@ -215,7 +242,10 @@ func L4proxyPolicySupported() error { // L4WfpProxySupported returns an error if the HCN verison does not support L4WfpProxy func L4WfpProxyPolicySupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.L4WfpProxy { return nil } @@ -224,7 +254,10 @@ func L4WfpProxyPolicySupported() error { // SetPolicySupported returns an error if the HCN version does not support SetPolicy. func SetPolicySupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.SetPolicy { return nil } @@ -233,7 +266,10 @@ func SetPolicySupported() error { // VxlanPortSupported returns an error if the HCN version does not support configuring the VXLAN TCP port. func VxlanPortSupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.VxlanPort { return nil } @@ -242,7 +278,10 @@ func VxlanPortSupported() error { // TierAclPolicySupported returns an error if the HCN version does not support configuring the TierAcl. func TierAclPolicySupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.TierAcl { return nil } diff --git a/hcn/hcnsupport.go b/hcn/hcnsupport.go index f15a5d34ee..cf8b54fefd 100644 --- a/hcn/hcnsupport.go +++ b/hcn/hcnsupport.go @@ -4,10 +4,17 @@ import ( "fmt" "sync" + "github.com/pkg/errors" "github.com/sirupsen/logrus" ) -var versionOnce sync.Once +var ( + // featuresOnce handles assigning the supported features and printing the supported info to stdout only once to avoid unnecessary work + // multiple times. + featuresOnce sync.Once + versionErr error + supportedFeatures SupportedFeatures +) // SupportedFeatures are the features provided by the Service. type SupportedFeatures struct { @@ -41,7 +48,59 @@ type ApiSupport struct { V2 bool `json:"V2"` } -// GetSupportedFeatures returns the features supported by the Service. +// GetCachedSupportedFeatures returns the features supported by the Service and an error if the query failed. If this has been called +// before it will return the supported features and error received from the first call. This can be used to optimize if many calls to the +// various hcn.IsXSupported methods need to be made. +func GetCachedSupportedFeatures() (SupportedFeatures, error) { + // Only query the HCN version and features supported once, instead of everytime this is invoked. The logs are useful to + // debug incidents where there's confusion on if a feature is supported on the host machine. The sync.Once helps to avoid redundant + // spam of these anytime a check needs to be made for if an HCN feature is supported. This is a common occurrence in kube-proxy + // for example. + featuresOnce.Do(func() { + globals, err := GetGlobals() + if err != nil { + // It's expected if this fails once, it should always fail. It should fail on pre 1803 builds for example. + versionErr = errors.Wrap(err, "failed to query HCN version number: this is expected on pre 1803 builds.") + } else { + supportedFeatures.Acl = AclFeatures{ + AclAddressLists: isFeatureSupported(globals.Version, HNSVersion1803), + AclNoHostRulePriority: isFeatureSupported(globals.Version, HNSVersion1803), + AclPortRanges: isFeatureSupported(globals.Version, HNSVersion1803), + AclRuleId: isFeatureSupported(globals.Version, HNSVersion1803), + } + + supportedFeatures.Api = ApiSupport{ + V2: isFeatureSupported(globals.Version, V2ApiSupport), + V1: true, // HNSCall is still available. + } + + supportedFeatures.RemoteSubnet = isFeatureSupported(globals.Version, RemoteSubnetVersion) + supportedFeatures.HostRoute = isFeatureSupported(globals.Version, HostRouteVersion) + supportedFeatures.DSR = isFeatureSupported(globals.Version, DSRVersion) + supportedFeatures.Slash32EndpointPrefixes = isFeatureSupported(globals.Version, Slash32EndpointPrefixesVersion) + supportedFeatures.AclSupportForProtocol252 = isFeatureSupported(globals.Version, AclSupportForProtocol252Version) + supportedFeatures.SessionAffinity = isFeatureSupported(globals.Version, SessionAffinityVersion) + supportedFeatures.IPv6DualStack = isFeatureSupported(globals.Version, IPv6DualStackVersion) + supportedFeatures.SetPolicy = isFeatureSupported(globals.Version, SetPolicyVersion) + supportedFeatures.VxlanPort = isFeatureSupported(globals.Version, VxlanPortVersion) + supportedFeatures.L4Proxy = isFeatureSupported(globals.Version, L4ProxyPolicyVersion) + supportedFeatures.L4WfpProxy = isFeatureSupported(globals.Version, L4WfpProxyPolicyVersion) + supportedFeatures.TierAcl = isFeatureSupported(globals.Version, TierAclPolicyVersion) + supportedFeatures.NetworkACL = isFeatureSupported(globals.Version, NetworkACLPolicyVersion) + supportedFeatures.NestedIpSet = isFeatureSupported(globals.Version, NestedIpSetVersion) + + logrus.WithFields(logrus.Fields{ + "version": fmt.Sprintf("%+v", globals.Version), + "supportedFeatures": fmt.Sprintf("%+v", supportedFeatures), + }).Info("HCN feature check") + } + }) + + return supportedFeatures, versionErr +} + +// GetSupportedFeatures returns the features supported by the Service. Prefer `GetCachedSupportedFeatures` as this method will query hns and validate +// every feature is supported on every invocation. func GetSupportedFeatures() SupportedFeatures { var features SupportedFeatures @@ -77,16 +136,10 @@ func GetSupportedFeatures() SupportedFeatures { features.L4WfpProxy = isFeatureSupported(globals.Version, L4WfpProxyPolicyVersion) features.TierAcl = isFeatureSupported(globals.Version, TierAclPolicyVersion) - // Only print the HCN version and features supported once, instead of everytime this is invoked. These logs are useful to - // debug incidents where there's confusion on if a feature is supported on the host machine. The sync.Once helps to avoid redundant - // spam of these anytime a check needs to be made for if an HCN feature is supported. This is a common occurrence in kubeproxy - // for example. - versionOnce.Do(func() { - logrus.WithFields(logrus.Fields{ - "version": fmt.Sprintf("%+v", globals.Version), - "supportedFeatures": fmt.Sprintf("%+v", features), - }).Info("HCN feature check") - }) + logrus.WithFields(logrus.Fields{ + "version": fmt.Sprintf("%+v", globals.Version), + "supportedFeatures": fmt.Sprintf("%+v", features), + }).Info("HCN feature check") return features } From 59bff3511f0055263dde6f7f971aba292fc7cc16 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Thu, 19 Aug 2021 03:51:58 -0700 Subject: [PATCH 3/5] PR feedback - Change versionErr -> featuresErr - Move work inside of the sync.Once out to its own function so we can use standard return err error handling and simply assign the output in GetCachedSupportedFeatures - Mark GetSupportedFeatures as deprecated. Signed-off-by: Daniel Canter (cherry picked from commit f78d0ffd3ad7ebe3073cd1be149af64ab6be6697) Signed-off-by: Daniel Canter --- hcn/hcnsupport.go | 63 +++++++++++++---------------------------------- 1 file changed, 17 insertions(+), 46 deletions(-) diff --git a/hcn/hcnsupport.go b/hcn/hcnsupport.go index cf8b54fefd..7e78167169 100644 --- a/hcn/hcnsupport.go +++ b/hcn/hcnsupport.go @@ -12,7 +12,7 @@ var ( // featuresOnce handles assigning the supported features and printing the supported info to stdout only once to avoid unnecessary work // multiple times. featuresOnce sync.Once - versionErr error + featuresErr error supportedFeatures SupportedFeatures ) @@ -57,60 +57,31 @@ func GetCachedSupportedFeatures() (SupportedFeatures, error) { // spam of these anytime a check needs to be made for if an HCN feature is supported. This is a common occurrence in kube-proxy // for example. featuresOnce.Do(func() { - globals, err := GetGlobals() - if err != nil { - // It's expected if this fails once, it should always fail. It should fail on pre 1803 builds for example. - versionErr = errors.Wrap(err, "failed to query HCN version number: this is expected on pre 1803 builds.") - } else { - supportedFeatures.Acl = AclFeatures{ - AclAddressLists: isFeatureSupported(globals.Version, HNSVersion1803), - AclNoHostRulePriority: isFeatureSupported(globals.Version, HNSVersion1803), - AclPortRanges: isFeatureSupported(globals.Version, HNSVersion1803), - AclRuleId: isFeatureSupported(globals.Version, HNSVersion1803), - } - - supportedFeatures.Api = ApiSupport{ - V2: isFeatureSupported(globals.Version, V2ApiSupport), - V1: true, // HNSCall is still available. - } - - supportedFeatures.RemoteSubnet = isFeatureSupported(globals.Version, RemoteSubnetVersion) - supportedFeatures.HostRoute = isFeatureSupported(globals.Version, HostRouteVersion) - supportedFeatures.DSR = isFeatureSupported(globals.Version, DSRVersion) - supportedFeatures.Slash32EndpointPrefixes = isFeatureSupported(globals.Version, Slash32EndpointPrefixesVersion) - supportedFeatures.AclSupportForProtocol252 = isFeatureSupported(globals.Version, AclSupportForProtocol252Version) - supportedFeatures.SessionAffinity = isFeatureSupported(globals.Version, SessionAffinityVersion) - supportedFeatures.IPv6DualStack = isFeatureSupported(globals.Version, IPv6DualStackVersion) - supportedFeatures.SetPolicy = isFeatureSupported(globals.Version, SetPolicyVersion) - supportedFeatures.VxlanPort = isFeatureSupported(globals.Version, VxlanPortVersion) - supportedFeatures.L4Proxy = isFeatureSupported(globals.Version, L4ProxyPolicyVersion) - supportedFeatures.L4WfpProxy = isFeatureSupported(globals.Version, L4WfpProxyPolicyVersion) - supportedFeatures.TierAcl = isFeatureSupported(globals.Version, TierAclPolicyVersion) - supportedFeatures.NetworkACL = isFeatureSupported(globals.Version, NetworkACLPolicyVersion) - supportedFeatures.NestedIpSet = isFeatureSupported(globals.Version, NestedIpSetVersion) - - logrus.WithFields(logrus.Fields{ - "version": fmt.Sprintf("%+v", globals.Version), - "supportedFeatures": fmt.Sprintf("%+v", supportedFeatures), - }).Info("HCN feature check") - } + supportedFeatures, featuresErr = getSupportedFeatures() }) - return supportedFeatures, versionErr + return supportedFeatures, featuresErr } -// GetSupportedFeatures returns the features supported by the Service. Prefer `GetCachedSupportedFeatures` as this method will query hns and validate -// every feature is supported on every invocation. +// Deprecated: Use GetCachedSupportedFeatures instead. +// GetSupportedFeatures returns the features supported by the Service. func GetSupportedFeatures() SupportedFeatures { - var features SupportedFeatures - - globals, err := GetGlobals() + features, err := GetCachedSupportedFeatures() if err != nil { // Expected on pre-1803 builds, all features will be false/unsupported - logrus.Debugf("Unable to obtain globals: %s", err) + logrus.Errorf("Unable to obtain globals: %s", err) return features } + return features +} +func getSupportedFeatures() (SupportedFeatures, error) { + var features SupportedFeatures + globals, err := GetGlobals() + if err != nil { + // It's expected if this fails once, it should always fail. It should fail on pre 1803 builds for example. + return SupportedFeatures{}, errors.Wrap(err, "failed to query HCN version number: this is expected on pre 1803 builds.") + } features.Acl = AclFeatures{ AclAddressLists: isFeatureSupported(globals.Version, HNSVersion1803), AclNoHostRulePriority: isFeatureSupported(globals.Version, HNSVersion1803), @@ -141,7 +112,7 @@ func GetSupportedFeatures() SupportedFeatures { "supportedFeatures": fmt.Sprintf("%+v", features), }).Info("HCN feature check") - return features + return features, nil } func isFeatureSupported(currentVersion Version, versionsSupported VersionRanges) bool { From 15276ce7677d9fd490b5420986e35cbe857a5fb0 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Thu, 19 Aug 2021 12:08:07 -0700 Subject: [PATCH 4/5] Fix hcn GetSupportedFeatures's error log msg Signed-off-by: Daniel Canter (cherry picked from commit 16435c703bed94918583e90811669ae449c08e94) Signed-off-by: Daniel Canter --- hcn/hcnsupport.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hcn/hcnsupport.go b/hcn/hcnsupport.go index 7e78167169..36c23ac3fb 100644 --- a/hcn/hcnsupport.go +++ b/hcn/hcnsupport.go @@ -69,7 +69,7 @@ func GetSupportedFeatures() SupportedFeatures { features, err := GetCachedSupportedFeatures() if err != nil { // Expected on pre-1803 builds, all features will be false/unsupported - logrus.Errorf("Unable to obtain globals: %s", err) + logrus.WithError(err).Errorf("unable to obtain supported features") return features } return features From b65b99375fc6eff4cd0ee68c1e83022d44cab31e Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Thu, 19 Aug 2021 13:24:02 -0700 Subject: [PATCH 5/5] Fix GetSupportedFeatures' comment to please godoc Signed-off-by: Daniel Canter (cherry picked from commit 57c5d6094e233bbb15f08b662e9eb3edafb0b131) Signed-off-by: Daniel Canter --- hcn/hcnsupport.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hcn/hcnsupport.go b/hcn/hcnsupport.go index 36c23ac3fb..64f9e3728b 100644 --- a/hcn/hcnsupport.go +++ b/hcn/hcnsupport.go @@ -63,8 +63,9 @@ func GetCachedSupportedFeatures() (SupportedFeatures, error) { return supportedFeatures, featuresErr } -// Deprecated: Use GetCachedSupportedFeatures instead. // GetSupportedFeatures returns the features supported by the Service. +// +// Deprecated: Use GetCachedSupportedFeatures instead. func GetSupportedFeatures() SupportedFeatures { features, err := GetCachedSupportedFeatures() if err != nil {