From 112b5e7a30d0cac219085a26fc3a8188bc471db9 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Wed, 18 Aug 2021 20:06:20 -0700 Subject: [PATCH 1/4] 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 --- hcn/hcn.go | 76 ++++++++++++++++++++++++++++++++++++---------- hcn/hcnsupport.go | 77 +++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 125 insertions(+), 28 deletions(-) diff --git a/hcn/hcn.go b/hcn/hcn.go index 9f6317cc05..b6678fb65f 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 } @@ -251,7 +290,10 @@ func TierAclPolicySupported() error { // NetworkACLPolicySupported returns an error if the HCN version does not support NetworkACLPolicy func NetworkACLPolicySupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.NetworkACL { return nil } @@ -260,14 +302,16 @@ func NetworkACLPolicySupported() error { // NestedIpSetSupported returns an error if the HCN version does not support NestedIpSet func NestedIpSetSupported() error { - supported := GetSupportedFeatures() + supported, err := GetCachedSupportedFeatures() + if err != nil { + return err + } if supported.NestedIpSet { return nil } return platformDoesNotSupportError("NestedIpSet") } - // RequestType are the different operations performed to settings. // Used to update the settings of Endpoint/Namespace objects. type RequestType string diff --git a/hcn/hcnsupport.go b/hcn/hcnsupport.go index 4be8df17db..250cf8ced3 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 { @@ -43,7 +50,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 @@ -81,16 +140,10 @@ func GetSupportedFeatures() SupportedFeatures { features.NetworkACL = isFeatureSupported(globals.Version, NetworkACLPolicyVersion) features.NestedIpSet = isFeatureSupported(globals.Version, NestedIpSetVersion) - // 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 f78d0ffd3ad7ebe3073cd1be149af64ab6be6697 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Thu, 19 Aug 2021 03:51:58 -0700 Subject: [PATCH 2/4] 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 --- hcn/hcnsupport.go | 63 +++++++++++++---------------------------------- 1 file changed, 17 insertions(+), 46 deletions(-) diff --git a/hcn/hcnsupport.go b/hcn/hcnsupport.go index 250cf8ced3..ac7b4c2021 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 ) @@ -59,60 +59,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), @@ -145,7 +116,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 16435c703bed94918583e90811669ae449c08e94 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Thu, 19 Aug 2021 12:08:07 -0700 Subject: [PATCH 3/4] Fix hcn GetSupportedFeatures's error log msg 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 ac7b4c2021..076344a15c 100644 --- a/hcn/hcnsupport.go +++ b/hcn/hcnsupport.go @@ -71,7 +71,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 57c5d6094e233bbb15f08b662e9eb3edafb0b131 Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Thu, 19 Aug 2021 13:24:02 -0700 Subject: [PATCH 4/4] Fix GetSupportedFeatures' comment to please godoc 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 076344a15c..bacb91feda 100644 --- a/hcn/hcnsupport.go +++ b/hcn/hcnsupport.go @@ -65,8 +65,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 {