From 901da441de0fcd24499df5dd9efd8ca89f18c18a Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 16 Jul 2019 22:06:37 -0400 Subject: [PATCH 1/4] Add a feature gate for legacy node-role behavior This gate will default to on in 1.16 to cover the behavior of the existing system, and then in the future default to off and then be removed once all consumers have migrated. --- pkg/features/kube_features.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index c0939e570be..896abbffb0f 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -172,6 +172,12 @@ const ( // Enable pods to set sysctls on a pod Sysctls featuregate.Feature = "Sysctls" + // owner @smarterclayton + // alpha: v1.16 + // + // Enable legacy behavior to vary cluster functionality on the node-role.kubernetes.io labels. On by default (legacy), will be turned off in 1.18. + LegacyNodeRoleBehavior featuregate.Feature = "LegacyNodeRoleBehavior" + // owner @brendandburns // alpha: v1.9 // @@ -570,5 +576,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS // features that enable backwards compatibility but are scheduled to be removed // ... - HPAScaleToZero: {Default: false, PreRelease: featuregate.Alpha}, + HPAScaleToZero: {Default: false, PreRelease: featuregate.Alpha}, + LegacyNodeRoleBehavior: {Default: true, PreRelease: featuregate.Alpha}, } From 0f49d892d55341438343689d05fa49fb9a9143b0 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 16 Jul 2019 22:07:38 -0400 Subject: [PATCH 2/4] Service controller should begin moving away from node-role labels The service load balancer controller should honor the LegacyNodeRoleBehavior feature gate for checks that use node-roles, switch to using a non alpha annotation behind the gate, and prepare to graduate to beta. --- pkg/controller/service/service_controller.go | 46 +++++++++++++------ .../service/service_controller_test.go | 34 ++++++++++++++ 2 files changed, 67 insertions(+), 13 deletions(-) diff --git a/pkg/controller/service/service_controller.go b/pkg/controller/service/service_controller.go index 08f02d8b559..e9c51c557d0 100644 --- a/pkg/controller/service/service_controller.go +++ b/pkg/controller/service/service_controller.go @@ -58,23 +58,38 @@ const ( minRetryDelay = 5 * time.Second maxRetryDelay = 300 * time.Second - // LabelNodeRoleMaster specifies that a node is a master - // It's copied over to kubeadm until it's merged in core: https://github.com/kubernetes/kubernetes/pull/39112 - LabelNodeRoleMaster = "node-role.kubernetes.io/master" + // labelNodeRoleMaster specifies that a node is a master. The use of this label within the + // controller is deprecated and only considered when the LegacyNodeRoleBehavior feature gate + // is on. + labelNodeRoleMaster = "node-role.kubernetes.io/master" - // LabelNodeRoleExcludeBalancer specifies that the node should be - // exclude from load balancers created by a cloud provider. - LabelNodeRoleExcludeBalancer = "alpha.service-controller.kubernetes.io/exclude-balancer" + // labelNodeRoleExcludeBalancer specifies that the node should not be considered as a target + // for external load-balancers which use nodes as a second hop (e.g. many cloud LBs which only + // understand nodes). For services that use externalTrafficPolicy=Local, this may mean that + // any backends on excluded nodes are not reachable by those external load-balancers. + // Implementations of this exclusion may vary based on provider. This label is honored starting + // in 1.16 when the ServiceNodeExclusion gate is on. + labelNodeRoleExcludeBalancer = "node.kubernetes.io/exclude-from-external-load-balancers" + + // labelAlphaNodeRoleExcludeBalancer specifies that the node should be + // exclude from load balancers created by a cloud provider. This label is deprecated and will + // be removed in 1.17. + labelAlphaNodeRoleExcludeBalancer = "alpha.service-controller.kubernetes.io/exclude-balancer" // serviceNodeExclusionFeature is the feature gate name that // enables nodes to exclude themselves from service load balancers // originated from: https://github.com/kubernetes/kubernetes/blob/28e800245e/pkg/features/kube_features.go#L178 serviceNodeExclusionFeature = "ServiceNodeExclusion" - // ServiceLoadBalancerFinalizerFeature is the feature gate name that + // serviceLoadBalancerFinalizerFeature is the feature gate name that // enables Finalizer Protection for Service LoadBalancers. // orginated from: https://github.com/kubernetes/kubernetes/blob/28e800245e/pkg/features/kube_features.go#L433 serviceLoadBalancerFinalizerFeature = "ServiceLoadBalancerFinalizer" + + // legacyNodeRoleBehaviro is the feature gate name that enables legacy + // behavior to vary cluster functionality on the node-role.kubernetes.io + // labels. + legacyNodeRoleBehaviorFeature = "LegacyNodeRoleBehavior" ) type cachedService struct { @@ -625,14 +640,19 @@ func getNodeConditionPredicate() corelisters.NodeConditionPredicate { return false } - // As of 1.6, we will taint the master, but not necessarily mark it unschedulable. - // Recognize nodes labeled as master, and filter them also, as we were doing previously. - if _, hasMasterRoleLabel := node.Labels[LabelNodeRoleMaster]; hasMasterRoleLabel { - return false + if utilfeature.DefaultFeatureGate.Enabled(legacyNodeRoleBehaviorFeature) { + // As of 1.6, we will taint the master, but not necessarily mark it unschedulable. + // Recognize nodes labeled as master, and filter them also, as we were doing previously. + if _, hasMasterRoleLabel := node.Labels[labelNodeRoleMaster]; hasMasterRoleLabel { + return false + } } - if utilfeature.DefaultFeatureGate.Enabled(serviceNodeExclusionFeature) { - if _, hasExcludeBalancerLabel := node.Labels[LabelNodeRoleExcludeBalancer]; hasExcludeBalancerLabel { + // Will be removed in 1.17 + if _, hasExcludeBalancerLabel := node.Labels[labelAlphaNodeRoleExcludeBalancer]; hasExcludeBalancerLabel { + return false + } + if _, hasExcludeBalancerLabel := node.Labels[labelNodeRoleExcludeBalancer]; hasExcludeBalancerLabel { return false } } diff --git a/pkg/controller/service/service_controller_test.go b/pkg/controller/service/service_controller_test.go index 1c605cffa85..93d8c482889 100644 --- a/pkg/controller/service/service_controller_test.go +++ b/pkg/controller/service/service_controller_test.go @@ -1396,3 +1396,37 @@ func TestPatchStatus(t *testing.T) { }) } } + +func Test_getNodeConditionPredicate(t *testing.T) { + validNodeStatus := v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: "Test"}}} + tests := []struct { + name string + + enableExclusion bool + enableLegacy bool + input *v1.Node + want bool + }{ + {want: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{}}}}, + {want: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeRoleMaster: ""}}}}, + {want: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeRoleExcludeBalancer: ""}}}}, + {want: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelAlphaNodeRoleExcludeBalancer: ""}}}}, + + {want: true, enableExclusion: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeRoleMaster: ""}}}}, + {want: true, enableLegacy: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeRoleExcludeBalancer: ""}}}}, + + {want: false, enableLegacy: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeRoleMaster: ""}}}}, + {want: false, enableExclusion: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelAlphaNodeRoleExcludeBalancer: ""}}}}, + {want: false, enableExclusion: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeRoleExcludeBalancer: ""}}}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, serviceNodeExclusionFeature, tt.enableExclusion)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, legacyNodeRoleBehaviorFeature, tt.enableLegacy)() + + if result := getNodeConditionPredicate()(tt.input); result != tt.want { + t.Errorf("getNodeConditionPredicate() = %v, want %v", result, tt.want) + } + }) + } +} From 2888e6e923588d542ff3c6b3641aed96c8712ddb Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 16 Jul 2019 22:24:21 -0400 Subject: [PATCH 3/4] Node lifecycle controller should use a label for excluding nodes The current mechanism for excluding "master" nodes based on node names is fragile and should be fixed by using a label exclusion similar to service load balancers. The legacy code path is preserved behind a defaulted-on gate and will be removed in the future. --- pkg/controller/nodelifecycle/BUILD | 1 - .../node_lifecycle_controller.go | 48 +++++++++++++++++-- .../node_lifecycle_controller_test.go | 34 ++++++++++++- pkg/features/kube_features.go | 7 +++ 4 files changed, 84 insertions(+), 6 deletions(-) diff --git a/pkg/controller/nodelifecycle/BUILD b/pkg/controller/nodelifecycle/BUILD index 34a02a89682..3f8fd555976 100644 --- a/pkg/controller/nodelifecycle/BUILD +++ b/pkg/controller/nodelifecycle/BUILD @@ -17,7 +17,6 @@ go_library( "//pkg/scheduler/api:go_default_library", "//pkg/util/metrics:go_default_library", "//pkg/util/node:go_default_library", - "//pkg/util/system:go_default_library", "//pkg/util/taints:go_default_library", "//staging/src/k8s.io/api/coordination/v1beta1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller.go b/pkg/controller/nodelifecycle/node_lifecycle_controller.go index 4a12a0440f1..490de709ddb 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller.go @@ -23,13 +23,14 @@ package nodelifecycle import ( "fmt" + "strings" "sync" "time" "k8s.io/klog" coordv1beta1 "k8s.io/api/coordination/v1beta1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -54,11 +55,11 @@ import ( "k8s.io/kubernetes/pkg/controller/nodelifecycle/scheduler" nodeutil "k8s.io/kubernetes/pkg/controller/util/node" "k8s.io/kubernetes/pkg/features" + kubefeatures "k8s.io/kubernetes/pkg/features" kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis" schedulerapi "k8s.io/kubernetes/pkg/scheduler/api" "k8s.io/kubernetes/pkg/util/metrics" utilnode "k8s.io/kubernetes/pkg/util/node" - "k8s.io/kubernetes/pkg/util/system" taintutils "k8s.io/kubernetes/pkg/util/taints" ) @@ -715,8 +716,8 @@ func (nc *Controller) monitorNodeHealth() error { continue } - // We do not treat a master node as a part of the cluster for network disruption checking. - if !system.IsMasterNode(node.Name) { + // Some nodes may be excluded from disruption checking + if !isNodeExcludedFromDisruptionChecks(node) { zoneToNodeConditions[utilnode.GetZoneKey(node)] = append(zoneToNodeConditions[utilnode.GetZoneKey(node)], currentReadyCondition) } @@ -806,6 +807,45 @@ func (nc *Controller) monitorNodeHealth() error { return nil } +// labelNodeDisruptionExclusion is a label on nodes that controls whether they are +// excluded from being considered for disruption checks by the node controller. +const labelNodeDisruptionExclusion = "node.kubernetes.io/exclude-disruption" + +func isNodeExcludedFromDisruptionChecks(node *v1.Node) bool { + // DEPRECATED: will be removed in 1.19 + if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.LegacyNodeRoleBehavior) { + if legacyIsMasterNode(node.Name) { + return true + } + } + if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.NodeDisruptionExclusion) { + if _, ok := node.Labels[labelNodeDisruptionExclusion]; ok { + return true + } + } + return false +} + +// legacyIsMasterNode returns true if given node is a registered master according +// to the logic historically used for this function. This code path is deprecated +// and the node disruption exclusion label should be used in the future. +// This code will not be allowed to update to use the node-role label, since +// node-roles may not be used for feature enablement. +// DEPRECATED: Will be removed in 1.19 +func legacyIsMasterNode(nodeName string) bool { + // We are trying to capture "master(-...)?$" regexp. + // However, using regexp.MatchString() results even in more than 35% + // of all space allocations in ControllerManager spent in this function. + // That's why we are trying to be a bit smarter. + if strings.HasSuffix(nodeName, "master") { + return true + } + if len(nodeName) >= 10 { + return strings.HasSuffix(nodeName[:len(nodeName)-3], "master-") + } + return false +} + // tryUpdateNodeHealth checks a given node's conditions and tries to update it. Returns grace period to // which given node is entitled, state of current and last observed Ready Condition, and an error if it occurred. func (nc *Controller) tryUpdateNodeHealth(node *v1.Node) (time.Duration, v1.NodeCondition, *v1.NodeCondition, error) { diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go index e5d417ad464..a4ff886b06f 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go @@ -23,7 +23,7 @@ import ( apps "k8s.io/api/apps/v1" coordv1beta1 "k8s.io/api/coordination/v1beta1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -3270,3 +3270,35 @@ func TestTryUpdateNodeHealth(t *testing.T) { }) } } + +func Test_isNodeExcludedFromDisruptionChecks(t *testing.T) { + validNodeStatus := v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: "Test"}}} + tests := []struct { + name string + + enableExclusion bool + enableLegacy bool + input *v1.Node + want bool + }{ + {want: false, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{}}}}, + {want: false, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Name: "master-abc"}}}, + {want: false, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeDisruptionExclusion: ""}}}}, + + {want: false, enableExclusion: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Name: "master-abc"}}}, + {want: false, enableLegacy: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeDisruptionExclusion: ""}}}}, + + {want: true, enableLegacy: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Name: "master-abc"}}}, + {want: true, enableExclusion: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeDisruptionExclusion: ""}}}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.NodeDisruptionExclusion, tt.enableExclusion)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LegacyNodeRoleBehavior, tt.enableLegacy)() + + if result := isNodeExcludedFromDisruptionChecks(tt.input); result != tt.want { + t.Errorf("isNodeExcludedFromDisruptionChecks() = %v, want %v", result, tt.want) + } + }) + } +} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 896abbffb0f..00c9f86d2f0 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -184,6 +184,12 @@ const ( // Enable nodes to exclude themselves from service load balancers ServiceNodeExclusion featuregate.Feature = "ServiceNodeExclusion" + // owner @smarterclayton + // alpha: v1.16 + // + // Enable nodes to exclude themselves from network disruption checks + NodeDisruptionExclusion featuregate.Feature = "NodeDisruptionExclusion" + // owner: @jsafrane // alpha: v1.9 // @@ -508,6 +514,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS CPUCFSQuotaPeriod: {Default: false, PreRelease: featuregate.Alpha}, TopologyManager: {Default: false, PreRelease: featuregate.Alpha}, ServiceNodeExclusion: {Default: false, PreRelease: featuregate.Alpha}, + NodeDisruptionExclusion: {Default: false, PreRelease: featuregate.Alpha}, MountContainers: {Default: false, PreRelease: featuregate.Alpha}, CSIDriverRegistry: {Default: true, PreRelease: featuregate.Beta}, CSINodeInfo: {Default: true, PreRelease: featuregate.Beta}, From a49a554211101f594e55e4b7d4574b535249db58 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 16 Jul 2019 22:29:39 -0400 Subject: [PATCH 4/4] Move the IsMasterNode function to tests and mark it Deprecated A future change will stop using this signal and instead use a label selector passed on creation. --- pkg/controller/.import-restrictions | 1 - pkg/util/BUILD | 1 - test/e2e/BUILD | 1 + test/e2e/framework/BUILD | 3 +-- test/e2e/framework/log/logger_test.go | 2 +- test/e2e/framework/metrics/BUILD | 2 +- test/e2e/framework/metrics/latencies.go | 4 ++-- test/e2e/framework/metrics/metrics_grabber.go | 4 ++-- test/e2e/framework/node/BUILD | 2 +- test/e2e/framework/node/resource.go | 4 ++-- test/e2e/framework/node/wait.go | 4 ++-- test/e2e/framework/resource_usage_gatherer.go | 10 +++++----- test/e2e/framework/util.go | 3 +-- {pkg/util => test/e2e}/system/BUILD | 12 ++++-------- {pkg/util => test/e2e}/system/system_utils.go | 8 +++++--- {pkg/util => test/e2e}/system/system_utils_test.go | 2 +- test/test_owners.csv | 1 - 17 files changed, 29 insertions(+), 35 deletions(-) rename {pkg/util => test/e2e}/system/BUILD (74%) rename {pkg/util => test/e2e}/system/system_utils.go (75%) rename {pkg/util => test/e2e}/system/system_utils_test.go (96%) diff --git a/pkg/controller/.import-restrictions b/pkg/controller/.import-restrictions index 9d5894434b1..5e3487e4770 100644 --- a/pkg/controller/.import-restrictions +++ b/pkg/controller/.import-restrictions @@ -244,7 +244,6 @@ "k8s.io/kubernetes/pkg/util/mount", "k8s.io/kubernetes/pkg/util/node", "k8s.io/kubernetes/pkg/util/slice", - "k8s.io/kubernetes/pkg/util/system", "k8s.io/kubernetes/pkg/util/taints", "k8s.io/kubernetes/pkg/volume", "k8s.io/kubernetes/pkg/volume/util", diff --git a/pkg/util/BUILD b/pkg/util/BUILD index 0373818fd4d..b280f705965 100644 --- a/pkg/util/BUILD +++ b/pkg/util/BUILD @@ -47,7 +47,6 @@ filegroup( "//pkg/util/selinux:all-srcs", "//pkg/util/slice:all-srcs", "//pkg/util/sysctl:all-srcs", - "//pkg/util/system:all-srcs", "//pkg/util/tail:all-srcs", "//pkg/util/taints:all-srcs", "//pkg/util/tolerations:all-srcs", diff --git a/test/e2e/BUILD b/test/e2e/BUILD index 348d404fd89..1e8b63d999d 100644 --- a/test/e2e/BUILD +++ b/test/e2e/BUILD @@ -113,6 +113,7 @@ filegroup( "//test/e2e/scheduling:all-srcs", "//test/e2e/servicecatalog:all-srcs", "//test/e2e/storage:all-srcs", + "//test/e2e/system:all-srcs", "//test/e2e/testing-manifests:all-srcs", "//test/e2e/ui:all-srcs", "//test/e2e/upgrades:all-srcs", diff --git a/test/e2e/framework/BUILD b/test/e2e/framework/BUILD index b3507596ad4..4e826b2bdca 100644 --- a/test/e2e/framework/BUILD +++ b/test/e2e/framework/BUILD @@ -34,7 +34,6 @@ go_library( "//pkg/apis/storage/v1/util:go_default_library", "//pkg/client/conditions:go_default_library", "//pkg/controller:go_default_library", - "//pkg/controller/service:go_default_library", "//pkg/features:go_default_library", "//pkg/kubelet/apis/config:go_default_library", "//pkg/kubelet/events:go_default_library", @@ -42,7 +41,6 @@ go_library( "//pkg/master/ports:go_default_library", "//pkg/scheduler/algorithm/predicates:go_default_library", "//pkg/scheduler/nodeinfo:go_default_library", - "//pkg/util/system:go_default_library", "//pkg/util/taints:go_default_library", "//pkg/version:go_default_library", "//pkg/volume/util:go_default_library", @@ -96,6 +94,7 @@ go_library( "//test/e2e/framework/ssh:go_default_library", "//test/e2e/framework/testfiles:go_default_library", "//test/e2e/manifest:go_default_library", + "//test/e2e/system:go_default_library", "//test/utils:go_default_library", "//test/utils/image:go_default_library", "//vendor/github.com/onsi/ginkgo:go_default_library", diff --git a/test/e2e/framework/log/logger_test.go b/test/e2e/framework/log/logger_test.go index 8a51ff51cfa..79c38f4770d 100644 --- a/test/e2e/framework/log/logger_test.go +++ b/test/e2e/framework/log/logger_test.go @@ -84,7 +84,7 @@ func TestFailureOutput(t *testing.T) { output: "INFO: before\nFAIL: hard-coded error\nUnexpected error:\n <*errors.errorString>: {\n s: \"an error with a long, useless description\",\n }\n an error with a long, useless description\noccurred\nINFO: after\nFAIL: true is never false either\nExpected\n : true\nto equal\n : false\n", failure: "hard-coded error\nUnexpected error:\n <*errors.errorString>: {\n s: \"an error with a long, useless description\",\n }\n an error with a long, useless description\noccurred", // TODO: should start with k8s.io/kubernetes/test/e2e/framework/log_test.glob..func1.4() - stack: "\tutil.go:1369\nk8s.io/kubernetes/test/e2e/framework.ExpectNoError()\n\tutil.go:1363\nk8s.io/kubernetes/test/e2e/framework/log_test.glob..func1.4()\n\tlogger_test.go:49\nk8s.io/kubernetes/vendor/github.com/onsi/ginkgo/internal/leafnodes.(*runner).runSync()\n\tlogger_test.go:65\n", + stack: "\tutil.go:1368\nk8s.io/kubernetes/test/e2e/framework.ExpectNoError()\n\tutil.go:1362\nk8s.io/kubernetes/test/e2e/framework/log_test.glob..func1.4()\n\tlogger_test.go:49\nk8s.io/kubernetes/vendor/github.com/onsi/ginkgo/internal/leafnodes.(*runner).runSync()\n\tlogger_test.go:65\n", }, testResult{ name: "[Top Level] log fails", diff --git a/test/e2e/framework/metrics/BUILD b/test/e2e/framework/metrics/BUILD index 2ca9b24eff9..c686fa8d211 100644 --- a/test/e2e/framework/metrics/BUILD +++ b/test/e2e/framework/metrics/BUILD @@ -30,7 +30,6 @@ go_library( "//pkg/kubelet/metrics:go_default_library", "//pkg/master/ports:go_default_library", "//pkg/scheduler/metrics:go_default_library", - "//pkg/util/system:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", @@ -38,6 +37,7 @@ go_library( "//test/e2e/framework/log:go_default_library", "//test/e2e/framework/ssh:go_default_library", "//test/e2e/perftype:go_default_library", + "//test/e2e/system:go_default_library", "//vendor/github.com/onsi/gomega:go_default_library", "//vendor/github.com/prometheus/common/expfmt:go_default_library", "//vendor/github.com/prometheus/common/model:go_default_library", diff --git a/test/e2e/framework/metrics/latencies.go b/test/e2e/framework/metrics/latencies.go index 18b6743a4f5..faf0a46028c 100644 --- a/test/e2e/framework/metrics/latencies.go +++ b/test/e2e/framework/metrics/latencies.go @@ -30,9 +30,9 @@ import ( clientset "k8s.io/client-go/kubernetes" "k8s.io/kubernetes/pkg/master/ports" schedulermetric "k8s.io/kubernetes/pkg/scheduler/metrics" - "k8s.io/kubernetes/pkg/util/system" e2elog "k8s.io/kubernetes/test/e2e/framework/log" e2essh "k8s.io/kubernetes/test/e2e/framework/ssh" + "k8s.io/kubernetes/test/e2e/system" "github.com/onsi/gomega" @@ -210,7 +210,7 @@ func sendRestRequestToScheduler(c clientset.Interface, op, provider, cloudMaster var masterRegistered = false for _, node := range nodes.Items { - if system.IsMasterNode(node.Name) { + if system.DeprecatedMightBeMasterNode(node.Name) { masterRegistered = true } } diff --git a/test/e2e/framework/metrics/metrics_grabber.go b/test/e2e/framework/metrics/metrics_grabber.go index d975eb3a0b3..c6b2efbdb8e 100644 --- a/test/e2e/framework/metrics/metrics_grabber.go +++ b/test/e2e/framework/metrics/metrics_grabber.go @@ -24,7 +24,7 @@ import ( clientset "k8s.io/client-go/kubernetes" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/master/ports" - "k8s.io/kubernetes/pkg/util/system" + "k8s.io/kubernetes/test/e2e/system" "k8s.io/klog" ) @@ -63,7 +63,7 @@ func NewMetricsGrabber(c clientset.Interface, ec clientset.Interface, kubelets b klog.Warning("Can't find any Nodes in the API server to grab metrics from") } for _, node := range nodeList.Items { - if system.IsMasterNode(node.Name) { + if system.DeprecatedMightBeMasterNode(node.Name) { registeredMaster = true masterName = node.Name break diff --git a/test/e2e/framework/node/BUILD b/test/e2e/framework/node/BUILD index 09080ea1c64..87c35cb4b62 100644 --- a/test/e2e/framework/node/BUILD +++ b/test/e2e/framework/node/BUILD @@ -12,7 +12,6 @@ go_library( "//pkg/controller/nodelifecycle:go_default_library", "//pkg/scheduler/algorithm/predicates:go_default_library", "//pkg/scheduler/nodeinfo:go_default_library", - "//pkg/util/system:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library", @@ -20,6 +19,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//test/e2e/framework/log:go_default_library", + "//test/e2e/system:go_default_library", "//test/utils:go_default_library", ], ) diff --git a/test/e2e/framework/node/resource.go b/test/e2e/framework/node/resource.go index 88328dbf325..3531e2462db 100644 --- a/test/e2e/framework/node/resource.go +++ b/test/e2e/framework/node/resource.go @@ -30,8 +30,8 @@ import ( nodectlr "k8s.io/kubernetes/pkg/controller/nodelifecycle" "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" - "k8s.io/kubernetes/pkg/util/system" e2elog "k8s.io/kubernetes/test/e2e/framework/log" + "k8s.io/kubernetes/test/e2e/system" testutils "k8s.io/kubernetes/test/utils" ) @@ -371,7 +371,7 @@ func GetMasterAndWorkerNodes(c clientset.Interface) (sets.String, *v1.NodeList, return nil, nil, fmt.Errorf("get nodes error: %s", err) } for _, n := range all.Items { - if system.IsMasterNode(n.Name) { + if system.DeprecatedMightBeMasterNode(n.Name) { masters.Insert(n.Name) } else if isNodeSchedulable(&n) && isNodeUntainted(&n) { nodes.Items = append(nodes.Items, n) diff --git a/test/e2e/framework/node/wait.go b/test/e2e/framework/node/wait.go index 3004cbd3125..9934c24687e 100644 --- a/test/e2e/framework/node/wait.go +++ b/test/e2e/framework/node/wait.go @@ -26,8 +26,8 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" - "k8s.io/kubernetes/pkg/util/system" e2elog "k8s.io/kubernetes/test/e2e/framework/log" + "k8s.io/kubernetes/test/e2e/system" testutils "k8s.io/kubernetes/test/utils" ) @@ -83,7 +83,7 @@ func WaitForTotalHealthy(c clientset.Interface, timeout time.Duration) error { } missingPodsPerNode = make(map[string][]string) for _, node := range nodes.Items { - if !system.IsMasterNode(node.Name) { + if !system.DeprecatedMightBeMasterNode(node.Name) { for _, requiredPod := range requiredPerNodePods { foundRequired := false for _, presentPod := range systemPodsPerNode[node.Name] { diff --git a/test/e2e/framework/resource_usage_gatherer.go b/test/e2e/framework/resource_usage_gatherer.go index b6615f76329..b97580699e3 100644 --- a/test/e2e/framework/resource_usage_gatherer.go +++ b/test/e2e/framework/resource_usage_gatherer.go @@ -27,14 +27,14 @@ import ( "text/tabwriter" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientset "k8s.io/client-go/kubernetes" - "k8s.io/kubernetes/pkg/util/system" e2ekubelet "k8s.io/kubernetes/test/e2e/framework/kubelet" e2elog "k8s.io/kubernetes/test/e2e/framework/log" e2emetrics "k8s.io/kubernetes/test/e2e/framework/metrics" + "k8s.io/kubernetes/test/e2e/system" ) // ResourceConstraint is a struct to hold constraints. @@ -280,10 +280,10 @@ func NewResourceUsageGatherer(c clientset.Interface, options ResourceGathererOpt } dnsNodes := make(map[string]bool) for _, pod := range pods.Items { - if (options.Nodes == MasterNodes) && !system.IsMasterNode(pod.Spec.NodeName) { + if (options.Nodes == MasterNodes) && !system.DeprecatedMightBeMasterNode(pod.Spec.NodeName) { continue } - if (options.Nodes == MasterAndDNSNodes) && !system.IsMasterNode(pod.Spec.NodeName) && pod.Labels["k8s-app"] != "kube-dns" { + if (options.Nodes == MasterAndDNSNodes) && !system.DeprecatedMightBeMasterNode(pod.Spec.NodeName) && pod.Labels["k8s-app"] != "kube-dns" { continue } for _, container := range pod.Status.InitContainerStatuses { @@ -303,7 +303,7 @@ func NewResourceUsageGatherer(c clientset.Interface, options ResourceGathererOpt } for _, node := range nodeList.Items { - if options.Nodes == AllNodes || system.IsMasterNode(node.Name) || dnsNodes[node.Name] { + if options.Nodes == AllNodes || system.DeprecatedMightBeMasterNode(node.Name) || dnsNodes[node.Name] { g.workerWg.Add(1) g.workers = append(g.workers, resourceGatherWorker{ c: c, diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index daf25b8f27b..b825aae8d4c 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -74,7 +74,6 @@ import ( podutil "k8s.io/kubernetes/pkg/api/v1/pod" "k8s.io/kubernetes/pkg/client/conditions" "k8s.io/kubernetes/pkg/controller" - "k8s.io/kubernetes/pkg/controller/service" "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/master/ports" "k8s.io/kubernetes/pkg/scheduler/algorithm/predicates" @@ -1974,7 +1973,7 @@ func WaitForAllNodesSchedulable(c clientset.Interface, timeout time.Duration) er } for i := range nodes.Items { node := &nodes.Items[i] - if _, hasMasterRoleLabel := node.ObjectMeta.Labels[service.LabelNodeRoleMaster]; hasMasterRoleLabel { + if _, hasMasterRoleLabel := node.ObjectMeta.Labels["node-role.kubernetes.io/master"]; hasMasterRoleLabel { // Kops clusters have masters with spec.unscheduable = false and // node-role.kubernetes.io/master NoSchedule taint. // Don't wait for them. diff --git a/pkg/util/system/BUILD b/test/e2e/system/BUILD similarity index 74% rename from pkg/util/system/BUILD rename to test/e2e/system/BUILD index 071d76f019b..7d4478f92e2 100644 --- a/pkg/util/system/BUILD +++ b/test/e2e/system/BUILD @@ -1,15 +1,10 @@ -package(default_visibility = ["//visibility:public"]) - -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", - "go_test", -) +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = ["system_utils.go"], - importpath = "k8s.io/kubernetes/pkg/util/system", + importpath = "k8s.io/kubernetes/test/e2e/system", + visibility = ["//visibility:public"], ) go_test( @@ -33,4 +28,5 @@ filegroup( name = "all-srcs", srcs = [":package-srcs"], tags = ["automanaged"], + visibility = ["//visibility:public"], ) diff --git a/pkg/util/system/system_utils.go b/test/e2e/system/system_utils.go similarity index 75% rename from pkg/util/system/system_utils.go rename to test/e2e/system/system_utils.go index 61832d95bc5..b845ae8463a 100644 --- a/pkg/util/system/system_utils.go +++ b/test/e2e/system/system_utils.go @@ -20,9 +20,11 @@ import ( "strings" ) -// IsMasterNode returns true if given node is a registered master. -// TODO: find a better way of figuring out if given node is a registered master. -func IsMasterNode(nodeName string) bool { +// DeprecatedMightBeMasterNode returns true if given node is a registered master. +// This code must not be updated to use node role labels, since node role labels +// may not change behavior of the system. +// DEPRECATED: use a label selector provided by test initialization. +func DeprecatedMightBeMasterNode(nodeName string) bool { // We are trying to capture "master(-...)?$" regexp. // However, using regexp.MatchString() results even in more than 35% // of all space allocations in ControllerManager spent in this function. diff --git a/pkg/util/system/system_utils_test.go b/test/e2e/system/system_utils_test.go similarity index 96% rename from pkg/util/system/system_utils_test.go rename to test/e2e/system/system_utils_test.go index 393a822a3ce..944e684bfa7 100644 --- a/pkg/util/system/system_utils_test.go +++ b/test/e2e/system/system_utils_test.go @@ -39,7 +39,7 @@ func TestIsMasterNode(t *testing.T) { for _, tc := range testCases { node := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: tc.input}} - res := IsMasterNode(node.Name) + res := DeprecatedMightBeMasterNode(node.Name) if res != tc.result { t.Errorf("case \"%s\": expected %t, got %t", tc.input, tc.result, res) } diff --git a/test/test_owners.csv b/test/test_owners.csv index 05d1948f23b..375e645e130 100644 --- a/test/test_owners.csv +++ b/test/test_owners.csv @@ -793,7 +793,6 @@ k8s.io/kubernetes/pkg/util/oom,vishh,0, k8s.io/kubernetes/pkg/util/parsers,derekwaynecarr,1, k8s.io/kubernetes/pkg/util/procfs,roberthbailey,1, k8s.io/kubernetes/pkg/util/slice,quinton-hoole,0, -k8s.io/kubernetes/pkg/util/system,mwielgus,0, k8s.io/kubernetes/pkg/util/tail,zmerlynn,1, k8s.io/kubernetes/pkg/util/taints,rrati,0, k8s.io/kubernetes/pkg/util/term,davidopp,1,