Merge pull request #119374 from danwinship/kep-3178-ga

move KEP-3178 IPTablesOwnershipCleanup to GA

cmd/kubelet/app/options/options.go

@@ -465,8 +465,10 @@ func AddKubeletConfigFlags(mainfs *pflag.FlagSet, c *kubeletconfig.KubeletConfig
 	fs.DurationVar(&c.CPUCFSQuotaPeriod.Duration, "cpu-cfs-quota-period", c.CPUCFSQuotaPeriod.Duration, "Sets CPU CFS quota period value, cpu.cfs_period_us, defaults to Linux Kernel default")
 	fs.BoolVar(&c.EnableControllerAttachDetach, "enable-controller-attach-detach", c.EnableControllerAttachDetach, "Enables the Attach/Detach controller to manage attachment/detachment of volumes scheduled to this node, and disables kubelet from executing any attach/detach operations")
 	fs.BoolVar(&c.MakeIPTablesUtilChains, "make-iptables-util-chains", c.MakeIPTablesUtilChains, "If true, kubelet will ensure iptables utility rules are present on host.")
-	fs.Int32Var(&c.IPTablesMasqueradeBit, "iptables-masquerade-bit", c.IPTablesMasqueradeBit, "The bit of the fwmark space to mark packets for SNAT. Must be within the range [0, 31]. Please match this parameter with corresponding parameter in kube-proxy.")
+	fs.Int32Var(&c.IPTablesMasqueradeBit, "iptables-masquerade-bit", c.IPTablesMasqueradeBit, "Has no effect; use kube-proxy parameters to configure the KUBE-MARK-MASQ chain.")
-	fs.Int32Var(&c.IPTablesDropBit, "iptables-drop-bit", c.IPTablesDropBit, "The bit of the fwmark space to mark packets for dropping. Must be within the range [0, 31].")
+	fs.MarkDeprecated("iptables-masquerade-bit", "This flag has no effect and will be removed in a future version.")
+	fs.Int32Var(&c.IPTablesDropBit, "iptables-drop-bit", c.IPTablesDropBit, "Has no effect; kubelet no longer creates a KUBE-MARK-DROP chain")
+	fs.MarkDeprecated("iptables-drop-bit", "This flag has no effect and will be removed in a future version.")
 	fs.StringVar(&c.ContainerLogMaxSize, "container-log-max-size", c.ContainerLogMaxSize, "<Warning: Beta feature> Set the maximum size (e.g. 10Mi) of container log file before it is rotated.")
 	fs.Int32Var(&c.ContainerLogMaxFiles, "container-log-max-files", c.ContainerLogMaxFiles, "<Warning: Beta feature> Set the maximum number of container log files that can be present for a container. The number must be >= 2.")
 	fs.StringSliceVar(&c.AllowedUnsafeSysctls, "allowed-unsafe-sysctls", c.AllowedUnsafeSysctls, "Comma-separated whitelist of unsafe sysctls or unsafe sysctl patterns (ending in *). Use these at your own risk.")

pkg/features/kube_features.go

@@ -376,6 +376,7 @@ const (
 	// kep: https://kep.k8s.io/3178
 	// alpha: v1.25
 	// beta: v1.27
+	// stable: v1.28
 	//
 	// Causes kubelet to no longer create legacy IPTables rules
 	IPTablesOwnershipCleanup featuregate.Feature = "IPTablesOwnershipCleanup"
@@ -1015,7 +1016,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	InTreePluginvSphereUnregister: {Default: false, PreRelease: featuregate.Alpha},
 
-	IPTablesOwnershipCleanup: {Default: true, PreRelease: featuregate.Beta},
+	IPTablesOwnershipCleanup: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.30
 
 	JobMutableNodeSchedulingDirectives: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29
 

pkg/generated/openapi/zz_generated.openapi.go

@@ -57925,21 +57925,21 @@ func schema_k8sio_kubelet_config_v1beta1_KubeletConfiguration(ref common.Referen
 					},
 					"makeIPTablesUtilChains": {
 						SchemaProps: spec.SchemaProps{
-							Description: "makeIPTablesUtilChains, if true, causes the Kubelet ensures a set of iptables rules are present on host. These rules will serve as utility rules for various components, e.g. kube-proxy. The rules will be created based on iptablesMasqueradeBit and iptablesDropBit. Default: true",
+							Description: "makeIPTablesUtilChains, if true, causes the Kubelet to create the KUBE-IPTABLES-HINT chain in iptables as a hint to other components about the configuration of iptables on the system. Default: true",
 							Type:        []string{"boolean"},
 							Format:      "",
 						},
 					},
 					"iptablesMasqueradeBit": {
 						SchemaProps: spec.SchemaProps{
-							Description: "iptablesMasqueradeBit is the bit of the iptables fwmark space to mark for SNAT. Values must be within the range [0, 31]. Must be different from other mark bits. Warning: Please match the value of the corresponding parameter in kube-proxy. Default: 14",
+							Description: "iptablesMasqueradeBit formerly controlled the creation of the KUBE-MARK-MASQ chain. Deprecated: no longer has any effect. Default: 14",
 							Type:        []string{"integer"},
 							Format:      "int32",
 						},
 					},
 					"iptablesDropBit": {
 						SchemaProps: spec.SchemaProps{
-							Description: "iptablesDropBit is the bit of the iptables fwmark space to mark for dropping packets. Values must be within the range [0, 31]. Must be different from other mark bits. Default: 15",
+							Description: "iptablesDropBit formerly controlled the creation of the KUBE-MARK-DROP chain. Deprecated: no longer has any effect. Default: 15",
 							Type:        []string{"integer"},
 							Format:      "int32",
 						},

pkg/kubelet/apis/config/types.go

@@ -319,17 +319,15 @@ type KubeletConfiguration struct {
 	// flags are not as it expects. Otherwise the Kubelet will attempt to modify
 	// kernel flags to match its expectation.
 	ProtectKernelDefaults bool
-	// If true, Kubelet ensures a set of iptables rules are present on host.
+	// If true, Kubelet creates the KUBE-IPTABLES-HINT chain in iptables as a hint to
-	// These rules will serve as utility for various components, e.g. kube-proxy.
+	// other components about the configuration of iptables on the system.
-	// The rules will be created based on IPTablesMasqueradeBit and IPTablesDropBit.
 	MakeIPTablesUtilChains bool
-	// iptablesMasqueradeBit is the bit of the iptables fwmark space to mark for SNAT
+	// iptablesMasqueradeBit formerly controlled the creation of the KUBE-MARK-MASQ
-	// Values must be within the range [0, 31]. Must be different from other mark bits.
+	// chain.
-	// Warning: Please match the value of the corresponding parameter in kube-proxy.
+	// Deprecated: no longer has any effect.
-	// TODO: clean up IPTablesMasqueradeBit in kube-proxy
 	IPTablesMasqueradeBit int32
-	// iptablesDropBit is the bit of the iptables fwmark space to mark for dropping packets.
+	// iptablesDropBit formerly controlled the creation of the KUBE-MARK-DROP chain.
-	// Values must be within the range [0, 31]. Must be different from other mark bits.
+	// Deprecated: no longer has any effect.
 	IPTablesDropBit int32
 	// featureGates is a map of feature names to bools that enable or disable alpha/experimental
 	// features. This field modifies piecemeal the built-in default values from

pkg/kubelet/kubelet.go

@@ -371,18 +371,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 		return nil, fmt.Errorf("invalid sync frequency %d", kubeCfg.SyncFrequency.Duration)
 	}
 
-	if kubeCfg.MakeIPTablesUtilChains {
-		if kubeCfg.IPTablesMasqueradeBit > 31 || kubeCfg.IPTablesMasqueradeBit < 0 {
-			return nil, fmt.Errorf("iptables-masquerade-bit is not valid. Must be within [0, 31]")
-		}
-		if kubeCfg.IPTablesDropBit > 31 || kubeCfg.IPTablesDropBit < 0 {
-			return nil, fmt.Errorf("iptables-drop-bit is not valid. Must be within [0, 31]")
-		}
-		if kubeCfg.IPTablesDropBit == kubeCfg.IPTablesMasqueradeBit {
-			return nil, fmt.Errorf("iptables-masquerade-bit and iptables-drop-bit must be different")
-		}
-	}
-
 	if utilfeature.DefaultFeatureGate.Enabled(features.DisableCloudProviders) && cloudprovider.IsDeprecatedInternal(cloudProvider) {
 		cloudprovider.DisableWarningForProvider(cloudProvider)
 		return nil, fmt.Errorf("cloud provider %q was specified, but built-in cloud providers are disabled. Please set --cloud-provider=external and migrate to an external cloud provider", cloudProvider)
@@ -561,8 +549,6 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 		clock:                          clock.RealClock{},
 		enableControllerAttachDetach:   kubeCfg.EnableControllerAttachDetach,
 		makeIPTablesUtilChains:         kubeCfg.MakeIPTablesUtilChains,
-		iptablesMasqueradeBit:          int(kubeCfg.IPTablesMasqueradeBit),
-		iptablesDropBit:                int(kubeCfg.IPTablesDropBit),
 		keepTerminatedPodVolumes:       keepTerminatedPodVolumes,
 		nodeStatusMaxImages:            nodeStatusMaxImages,
 		tracer:                         tracer,
@@ -1276,12 +1262,6 @@ type Kubelet struct {
 	// config iptables util rules
 	makeIPTablesUtilChains bool
 
-	// The bit of the fwmark space to mark packets for SNAT.
-	iptablesMasqueradeBit int
-
-	// The bit of the fwmark space to mark packets for dropping.
-	iptablesDropBit int
-
 	// The AppArmor validator for checking whether AppArmor is supported.
 	appArmorValidator apparmor.Validator
 

pkg/kubelet/kubelet_network_linux.go

@@ -20,13 +20,10 @@ limitations under the License.
 package kubelet
 
 import (
-	"fmt"
 	"time"
 
 	"k8s.io/apimachinery/pkg/util/wait"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
-	"k8s.io/kubernetes/pkg/features"
 	utiliptables "k8s.io/kubernetes/pkg/util/iptables"
 	utilexec "k8s.io/utils/exec"
 )
@@ -36,16 +33,6 @@ const (
 	// or iptables-nft indicates which version of iptables the system is using
 	KubeIPTablesHintChain utiliptables.Chain = "KUBE-IPTABLES-HINT"
 
-	// KubeMarkMasqChain is the mark-for-masquerade chain
-	// TODO: clean up this logic in kube-proxy
-	KubeMarkMasqChain utiliptables.Chain = "KUBE-MARK-MASQ"
-
-	// KubeMarkDropChain is the mark-for-drop chain
-	KubeMarkDropChain utiliptables.Chain = "KUBE-MARK-DROP"
-
-	// KubePostroutingChain is kubernetes postrouting rules
-	KubePostroutingChain utiliptables.Chain = "KUBE-POSTROUTING"
-
 	// KubeFirewallChain is kubernetes firewall rules
 	KubeFirewallChain utiliptables.Chain = "KUBE-FIREWALL"
 )
@@ -74,8 +61,7 @@ func (kl *Kubelet) initNetworkUtil() {
 }
 
 // syncIPTablesRules ensures the KUBE-IPTABLES-HINT chain exists, and the martian packet
-// protection rule is installed. If the IPTablesOwnershipCleanup feature gate is disabled
+// protection rule is installed.
-// it will also synchronize additional deprecated iptables rules.
 func (kl *Kubelet) syncIPTablesRules(iptClient utiliptables.Interface) bool {
 	// Create hint chain so other components can see whether we are using iptables-legacy
 	// or iptables-nft.
@@ -125,102 +111,5 @@ func (kl *Kubelet) syncIPTablesRules(iptClient utiliptables.Interface) bool {
 		}
 	}
 
-	if !utilfeature.DefaultFeatureGate.Enabled(features.IPTablesOwnershipCleanup) {
-		ok := kl.syncIPTablesRulesDeprecated(iptClient)
-		if !ok {
-			return false
-		}
-	}
-
 	return true
 }
-
-// syncIPTablesRulesDeprecated ensures deprecated iptables rules are present:
-//  1. In nat table, KUBE-MARK-DROP rule to mark connections for dropping
-//     Marked connection will be drop on INPUT/OUTPUT Chain in filter table
-//  2. In nat table, KUBE-MARK-MASQ rule to mark connections for SNAT
-//     Marked connection will get SNAT on POSTROUTING Chain in nat table
-func (kl *Kubelet) syncIPTablesRulesDeprecated(iptClient utiliptables.Interface) bool {
-	// Setup KUBE-MARK-DROP rules
-	dropMark := getIPTablesMark(kl.iptablesDropBit)
-	if _, err := iptClient.EnsureChain(utiliptables.TableNAT, KubeMarkDropChain); err != nil {
-		klog.ErrorS(err, "Failed to ensure that KUBE-MARK-DROP chain exists")
-		return false
-	}
-	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubeMarkDropChain, "-j", "MARK", "--or-mark", dropMark); err != nil {
-		klog.ErrorS(err, "Failed to ensure that KUBE-MARK-DROP rule exists")
-		return false
-	}
-	if _, err := iptClient.EnsureChain(utiliptables.TableFilter, KubeFirewallChain); err != nil {
-		klog.ErrorS(err, "Failed to ensure that KUBE-FIREWALL chain exists")
-		return false
-	}
-	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableFilter, KubeFirewallChain,
-		"-m", "comment", "--comment", "kubernetes firewall for dropping marked packets",
-		"-m", "mark", "--mark", fmt.Sprintf("%s/%s", dropMark, dropMark),
-		"-j", "DROP"); err != nil {
-		klog.ErrorS(err, "Failed to ensure that KUBE-FIREWALL rule exists")
-		return false
-	}
-
-	// Setup KUBE-MARK-MASQ rules
-	masqueradeMark := getIPTablesMark(kl.iptablesMasqueradeBit)
-	if _, err := iptClient.EnsureChain(utiliptables.TableNAT, KubeMarkMasqChain); err != nil {
-		klog.ErrorS(err, "Failed to ensure that KUBE-MARK-MASQ chain exists")
-		return false
-	}
-	if _, err := iptClient.EnsureChain(utiliptables.TableNAT, KubePostroutingChain); err != nil {
-		klog.ErrorS(err, "Failed to ensure that KUBE-POSTROUTING chain exists")
-		return false
-	}
-	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubeMarkMasqChain, "-j", "MARK", "--or-mark", masqueradeMark); err != nil {
-		klog.ErrorS(err, "Failed to ensure that KUBE-MARK-MASQ rule exists")
-		return false
-	}
-	if _, err := iptClient.EnsureRule(utiliptables.Prepend, utiliptables.TableNAT, utiliptables.ChainPostrouting,
-		"-m", "comment", "--comment", "kubernetes postrouting rules", "-j", string(KubePostroutingChain)); err != nil {
-		klog.ErrorS(err, "Failed to ensure that POSTROUTING chain jumps to KUBE-POSTROUTING")
-		return false
-	}
-
-	// Set up KUBE-POSTROUTING to unmark and masquerade marked packets
-
-	// NOTE: kube-proxy (in iptables and ipvs modes) creates identical copies of these
-	// rules. If you want to change these rules in the future, you MUST do so in a way
-	// that will interoperate correctly with skewed versions of the rules created by
-	// kube-proxy.
-
-	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubePostroutingChain,
-		"-m", "mark", "!", "--mark", fmt.Sprintf("%s/%s", masqueradeMark, masqueradeMark),
-		"-j", "RETURN"); err != nil {
-		klog.ErrorS(err, "Failed to ensure first masquerading rule exists")
-		return false
-	}
-	// Clear the mark to avoid re-masquerading if the packet re-traverses the network stack.
-	// We know the mark bit is currently set so we can use --xor-mark to clear it (without needing
-	// to Sprintf another bitmask).
-	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubePostroutingChain,
-		"-j", "MARK", "--xor-mark", masqueradeMark); err != nil {
-		klog.ErrorS(err, "Failed to ensure second masquerading rule exists")
-		return false
-	}
-	masqRule := []string{
-		"-m", "comment", "--comment", "kubernetes service traffic requiring SNAT",
-		"-j", "MASQUERADE",
-	}
-	if iptClient.HasRandomFully() {
-		masqRule = append(masqRule, "--random-fully")
-	}
-	if _, err := iptClient.EnsureRule(utiliptables.Append, utiliptables.TableNAT, KubePostroutingChain, masqRule...); err != nil {
-		klog.ErrorS(err, "Failed to ensure third masquerading rule exists")
-		return false
-	}
-
-	return true
-}
-
-// getIPTablesMark returns the fwmark given the bit
-func getIPTablesMark(bit int) string {
-	value := 1 << uint(bit)
-	return fmt.Sprintf("%#08x", value)
-}

pkg/kubelet/kubelet_network_test.go

@@ -1,46 +0,0 @@
-//go:build linux
-// +build linux
-
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package kubelet
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestGetIPTablesMark(t *testing.T) {
-	tests := []struct {
-		bit    int
-		expect string
-	}{
-		{
-			14,
-			"0x00004000",
-		},
-		{
-			15,
-			"0x00008000",
-		},
-	}
-	for _, tc := range tests {
-		res := getIPTablesMark(tc.bit)
-		assert.Equal(t, tc.expect, res, "input %d", tc.bit)
-	}
-}

pkg/proxy/iptables/proxier.go

@@ -370,6 +370,7 @@ var iptablesJumpChains = []iptablesJumpChain{
 	{utiliptables.TableFilter, kubeProxyFirewallChain, utiliptables.ChainForward, "kubernetes load balancer firewall", []string{"-m", "conntrack", "--ctstate", "NEW"}},
 	{utiliptables.TableNAT, kubeServicesChain, utiliptables.ChainOutput, "kubernetes service portals", nil},
 	{utiliptables.TableNAT, kubeServicesChain, utiliptables.ChainPrerouting, "kubernetes service portals", nil},
+	{utiliptables.TableNAT, kubePostroutingChain, utiliptables.ChainPostrouting, "kubernetes postrouting rules", nil},
 }
 
 // Duplicates of chains created in pkg/kubelet/kubelet_network_linux.go; we create these
@@ -377,10 +378,6 @@ var iptablesJumpChains = []iptablesJumpChain{
 var iptablesKubeletJumpChains = []iptablesJumpChain{
 	{utiliptables.TableFilter, kubeletFirewallChain, utiliptables.ChainInput, "", nil},
 	{utiliptables.TableFilter, kubeletFirewallChain, utiliptables.ChainOutput, "", nil},
-
-	// Move this to iptablesJumpChains once IPTablesOwnershipCleanup is GA and kubelet
-	// no longer creates this chain,
-	{utiliptables.TableNAT, kubePostroutingChain, utiliptables.ChainPostrouting, "kubernetes postrouting rules", nil},
 }
 
 // When chains get removed from iptablesJumpChains, add them here so they get cleaned up
@@ -868,11 +865,6 @@ func (proxier *Proxier) syncProxyRules() {
 	// this so that it is easier to flush and change, for example if the mark
 	// value should ever change.
 
-	// NOTE: kubelet creates identical copies of these rules. If you want to change
-	// these rules in the future, you MUST do so in a way that will interoperate
-	// correctly with skewed versions of the rules created by kubelet. (Remove this
-	// comment once IPTablesOwnershipCleanup is GA.)
-
 	proxier.natRules.Write(
 		"-A", string(kubePostroutingChain),
 		"-m", "mark", "!", "--mark", fmt.Sprintf("%s/%s", proxier.masqueradeMark, proxier.masqueradeMark),

pkg/proxy/ipvs/proxier.go

@@ -1693,11 +1693,6 @@ func (proxier *Proxier) writeIptablesRules() {
 	// this so that it is easier to flush and change, for example if the mark
 	// value should ever change.
 
-	// NOTE: kubelet creates identical copies of these rules. If you want to change
-	// these rules in the future, you MUST do so in a way that will interoperate
-	// correctly with skewed versions of the rules created by kubelet. (Remove this
-	// comment once IPTablesOwnershipCleanup is GA.)
-
 	proxier.natRules.Write(
 		"-A", string(kubePostroutingChain),
 		"-m", "mark", "!", "--mark", fmt.Sprintf("%s/%s", proxier.masqueradeMark, proxier.masqueradeMark),

staging/src/k8s.io/kubelet/config/v1beta1/types.go

@@ -548,22 +548,20 @@ type KubeletConfiguration struct {
 	// Default: false
 	// +optional
 	ProtectKernelDefaults bool `json:"protectKernelDefaults,omitempty"`
-	// makeIPTablesUtilChains, if true, causes the Kubelet ensures a set of iptables rules
+	// makeIPTablesUtilChains, if true, causes the Kubelet to create the
-	// are present on host.
+	// KUBE-IPTABLES-HINT chain in iptables as a hint to other components about the
-	// These rules will serve as utility rules for various components, e.g. kube-proxy.
+	// configuration of iptables on the system.
-	// The rules will be created based on iptablesMasqueradeBit and iptablesDropBit.
 	// Default: true
 	// +optional
 	MakeIPTablesUtilChains *bool `json:"makeIPTablesUtilChains,omitempty"`
-	// iptablesMasqueradeBit is the bit of the iptables fwmark space to mark for SNAT.
+	// iptablesMasqueradeBit formerly controlled the creation of the KUBE-MARK-MASQ
-	// Values must be within the range [0, 31]. Must be different from other mark bits.
+	// chain.
-	// Warning: Please match the value of the corresponding parameter in kube-proxy.
+	// Deprecated: no longer has any effect.
-	// TODO: clean up IPTablesMasqueradeBit in kube-proxy.
 	// Default: 14
 	// +optional
 	IPTablesMasqueradeBit *int32 `json:"iptablesMasqueradeBit,omitempty"`
-	// iptablesDropBit is the bit of the iptables fwmark space to mark for dropping packets.
+	// iptablesDropBit formerly controlled the creation of the KUBE-MARK-DROP chain.
-	// Values must be within the range [0, 31]. Must be different from other mark bits.
+	// Deprecated: no longer has any effect.
 	// Default: 15
 	// +optional
 	IPTablesDropBit *int32 `json:"iptablesDropBit,omitempty"`