From 5b8ad3f7c5f1e00261b481036955adff03022440 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 6 Jun 2017 10:27:35 -0500 Subject: [PATCH] kubelet: remove unused bandwidth shaping teardown code Since v1.5 and the removal of --configure-cbr0: 0800df74abab11ba43ac53b345653e4b814ed77b "Remove the legacy networking mode --configure-cbr0" kubelet hasn't done any shaping operations internally. They have all been delegated to network plugins like kubenet or external CNI plugins. But some shaping code was still left in kubelet, so remove it now that it's unused. --- pkg/kubelet/BUILD | 2 - pkg/kubelet/kubelet.go | 27 ------- pkg/kubelet/kubelet_network.go | 47 ----------- pkg/kubelet/kubelet_network_test.go | 80 ------------------- pkg/kubelet/kubelet_pods.go | 6 -- .../network/kubenet/kubenet_linux_test.go | 2 +- 6 files changed, 1 insertion(+), 163 deletions(-) diff --git a/pkg/kubelet/BUILD b/pkg/kubelet/BUILD index 9880249c4e3..2f9d71700e0 100644 --- a/pkg/kubelet/BUILD +++ b/pkg/kubelet/BUILD @@ -96,7 +96,6 @@ go_library( "//pkg/security/apparmor:go_default_library", "//pkg/securitycontext:go_default_library", "//pkg/util:go_default_library", - "//pkg/util/bandwidth:go_default_library", "//pkg/util/dbus:go_default_library", "//pkg/util/exec:go_default_library", "//pkg/util/io:go_default_library", @@ -201,7 +200,6 @@ go_test( "//pkg/kubelet/util/queue:go_default_library", "//pkg/kubelet/util/sliceutils:go_default_library", "//pkg/kubelet/volumemanager:go_default_library", - "//pkg/util/bandwidth:go_default_library", "//pkg/util/mount:go_default_library", "//pkg/version:go_default_library", "//pkg/volume:go_default_library", diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index bdbf2aaddaf..b969f7830b9 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -102,7 +102,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/util/sliceutils" "k8s.io/kubernetes/pkg/kubelet/volumemanager" "k8s.io/kubernetes/pkg/security/apparmor" - "k8s.io/kubernetes/pkg/util/bandwidth" utildbus "k8s.io/kubernetes/pkg/util/dbus" utilexec "k8s.io/kubernetes/pkg/util/exec" kubeio "k8s.io/kubernetes/pkg/util/io" @@ -1027,10 +1026,6 @@ type Kubelet struct { // clusterDomain and clusterDNS. resolverConfig string - // Optionally shape the bandwidth of a pod - // TODO: remove when kubenet plugin is ready - shaper bandwidth.BandwidthShaper - // Information about the ports which are opened by daemons on Node running this Kubelet server. daemonEndpoints *v1.NodeDaemonEndpoints @@ -1632,28 +1627,6 @@ func (kl *Kubelet) syncPod(o syncPodOptions) error { return err } - // early successful exit if pod is not bandwidth-constrained - if !kl.shapingEnabled() { - return nil - } - - // Update the traffic shaping for the pod's ingress and egress limits - ingress, egress, err := bandwidth.ExtractPodBandwidthResources(pod.Annotations) - if err != nil { - return err - } - if egress != nil || ingress != nil { - if kubecontainer.IsHostNetworkPod(pod) { - kl.recorder.Event(pod, v1.EventTypeWarning, events.HostNetworkNotSupported, "Bandwidth shaping is not currently supported on the host network") - } else if kl.shaper != nil { - if len(apiPodStatus.PodIP) > 0 { - err = kl.shaper.ReconcileCIDR(fmt.Sprintf("%s/32", apiPodStatus.PodIP), egress, ingress) - } - } else { - kl.recorder.Event(pod, v1.EventTypeWarning, events.UndefinedShaper, "Pod requests bandwidth shaping, but the shaper is undefined") - } - } - return nil } diff --git a/pkg/kubelet/kubelet_network.go b/pkg/kubelet/kubelet_network.go index 5ac13653ac5..8d8b7472eb3 100644 --- a/pkg/kubelet/kubelet_network.go +++ b/pkg/kubelet/kubelet_network.go @@ -25,10 +25,8 @@ import ( "github.com/golang/glog" "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/apis/componentconfig" "k8s.io/kubernetes/pkg/kubelet/network" - "k8s.io/kubernetes/pkg/util/bandwidth" utiliptables "k8s.io/kubernetes/pkg/util/iptables" ) @@ -244,51 +242,6 @@ func (kl *Kubelet) parseResolvConf(reader io.Reader) (nameservers []string, sear return nameservers, searches, nil } -// cleanupBandwidthLimits updates the status of bandwidth-limited containers -// and ensures that only the appropriate CIDRs are active on the node. -func (kl *Kubelet) cleanupBandwidthLimits(allPods []*v1.Pod) error { - if kl.shaper == nil { - return nil - } - currentCIDRs, err := kl.shaper.GetCIDRs() - if err != nil { - return err - } - possibleCIDRs := sets.String{} - for ix := range allPods { - pod := allPods[ix] - ingress, egress, err := bandwidth.ExtractPodBandwidthResources(pod.Annotations) - if err != nil { - return err - } - if ingress == nil && egress == nil { - glog.V(8).Infof("Not a bandwidth limited container...") - continue - } - status, found := kl.statusManager.GetPodStatus(pod.UID) - if !found { - // TODO(random-liu): Cleanup status get functions. (issue #20477) - s, err := kl.containerRuntime.GetPodStatus(pod.UID, pod.Name, pod.Namespace) - if err != nil { - return err - } - status = kl.generateAPIPodStatus(pod, s) - } - if status.Phase == v1.PodRunning { - possibleCIDRs.Insert(fmt.Sprintf("%s/32", status.PodIP)) - } - } - for _, cidr := range currentCIDRs { - if !possibleCIDRs.Has(cidr) { - glog.V(2).Infof("Removing CIDR: %s (%v)", cidr, possibleCIDRs) - if err := kl.shaper.Reset(cidr); err != nil { - return err - } - } - } - return nil -} - // syncNetworkStatus updates the network state func (kl *Kubelet) syncNetworkStatus() { // For cri integration, network state will be updated in updateRuntimeUp, diff --git a/pkg/kubelet/kubelet_network_test.go b/pkg/kubelet/kubelet_network_test.go index dc68b882681..5de1c582eda 100644 --- a/pkg/kubelet/kubelet_network_test.go +++ b/pkg/kubelet/kubelet_network_test.go @@ -26,7 +26,6 @@ import ( "github.com/stretchr/testify/require" "k8s.io/api/core/v1" "k8s.io/client-go/tools/record" - "k8s.io/kubernetes/pkg/util/bandwidth" ) func TestNodeIPParam(t *testing.T) { @@ -184,85 +183,6 @@ func TestComposeDNSSearch(t *testing.T) { } } -func TestCleanupBandwidthLimits(t *testing.T) { - testPod := func(name, ingress string) *v1.Pod { - pod := podWithUidNameNs("", name, "") - - if len(ingress) != 0 { - pod.Annotations["kubernetes.io/ingress-bandwidth"] = ingress - } - - return pod - } - - // TODO(random-liu): We removed the test case for pod status not cached here. We should add a higher - // layer status getter function and test that function instead. - tests := []struct { - status *v1.PodStatus - pods []*v1.Pod - inputCIDRs []string - expectResetCIDRs []string - name string - }{ - { - status: &v1.PodStatus{ - PodIP: "1.2.3.4", - Phase: v1.PodRunning, - }, - pods: []*v1.Pod{ - testPod("foo", "10M"), - testPod("bar", ""), - }, - inputCIDRs: []string{"1.2.3.4/32", "2.3.4.5/32", "5.6.7.8/32"}, - expectResetCIDRs: []string{"2.3.4.5/32", "5.6.7.8/32"}, - name: "pod running", - }, - { - status: &v1.PodStatus{ - PodIP: "1.2.3.4", - Phase: v1.PodFailed, - }, - pods: []*v1.Pod{ - testPod("foo", "10M"), - testPod("bar", ""), - }, - inputCIDRs: []string{"1.2.3.4/32", "2.3.4.5/32", "5.6.7.8/32"}, - expectResetCIDRs: []string{"1.2.3.4/32", "2.3.4.5/32", "5.6.7.8/32"}, - name: "pod not running", - }, - { - status: &v1.PodStatus{ - PodIP: "1.2.3.4", - Phase: v1.PodFailed, - }, - pods: []*v1.Pod{ - testPod("foo", ""), - testPod("bar", ""), - }, - inputCIDRs: []string{"1.2.3.4/32", "2.3.4.5/32", "5.6.7.8/32"}, - expectResetCIDRs: []string{"1.2.3.4/32", "2.3.4.5/32", "5.6.7.8/32"}, - name: "no bandwidth limits", - }, - } - for _, test := range tests { - shaper := &bandwidth.FakeShaper{ - CIDRs: test.inputCIDRs, - } - - testKube := newTestKubelet(t, false /* controllerAttachDetachEnabled */) - defer testKube.Cleanup() - testKube.kubelet.shaper = shaper - - for _, pod := range test.pods { - testKube.kubelet.statusManager.SetPodStatus(pod, *test.status) - } - - err := testKube.kubelet.cleanupBandwidthLimits(test.pods) - assert.NoError(t, err, "test [%s]", test.name) - assert.EqualValues(t, test.expectResetCIDRs, shaper.ResetCIDRs, "test[%s]", test.name) - } -} - func TestGetIPTablesMark(t *testing.T) { tests := []struct { bit int diff --git a/pkg/kubelet/kubelet_pods.go b/pkg/kubelet/kubelet_pods.go index 2784b433f1c..e0faebf4574 100644 --- a/pkg/kubelet/kubelet_pods.go +++ b/pkg/kubelet/kubelet_pods.go @@ -893,12 +893,6 @@ func (kl *Kubelet) HandlePodCleanups() error { // Remove any orphaned mirror pods. kl.podManager.DeleteOrphanedMirrorPods() - // Clear out any old bandwidth rules - err = kl.cleanupBandwidthLimits(allPods) - if err != nil { - glog.Errorf("Failed cleaning up bandwidth limits: %v", err) - } - // Remove any cgroups in the hierarchy for pods that are no longer running. if kl.cgroupsPerQOS { kl.cleanupOrphanedPodCgroups(cgroupPods, activePods) diff --git a/pkg/kubelet/network/kubenet/kubenet_linux_test.go b/pkg/kubelet/network/kubenet/kubenet_linux_test.go index c3fad8ca3f2..2e5f5a42dfb 100644 --- a/pkg/kubelet/network/kubenet/kubenet_linux_test.go +++ b/pkg/kubelet/network/kubenet/kubenet_linux_test.go @@ -125,7 +125,7 @@ func TestGetPodNetworkStatus(t *testing.T) { } } -// TestTeardownBeforeSetUp tests that a `TearDown` call does call +// TestTeardownCallsShaper tests that a `TearDown` call does call // `shaper.Reset` func TestTeardownCallsShaper(t *testing.T) { fexec := &exec.FakeExec{