Skip to content

Commit 5c8a578

Browse files
sridhargaddamsridhargaddam
authored
Support safe migration from iptables to nftables in Ambient (#58354)
When an existing Istio Ambient deployment using the iptables backend is upgraded to the nftables backend, IstioCNI shouldn’t switch to nftables silently. Doing so leaves stale iptables rules/IPsets on the host. If this happens along with reconcileIptablesOnStartup setting, the pod network namespaces end up with both nftables rules and the old iptables rules. In both cases, the stale iptables rules can cause issues until the node is rebooted. This PR updates the IstioCNI initialization code to detect any iptables artifacts on the host. If it finds them, it overrides the nftables setting, keeps using the iptables backend, and logs a message telling users to reboot the node to complete the migration. This allows safe upgrades with no pod disruption in Ambient mode. After a reboot, the old iptables artifacts are cleared and pods come up with clean namespaces, completing the migration automatically. Fixes: istio/istio#58353 Signed-off-by: Sridhar Gaddam <[email protected]>
1 parent 5245b4c commit 5c8a578

File tree

4 files changed

+291
-3
lines changed

4 files changed

+291
-3
lines changed

‎cni/pkg/nodeagent/detect_artifacts_linux.go‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
// Copyright Istio Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package nodeagent
16+
17+
import (
18+
"fmt"
19+
"strings"
20+
21+
"github.com/vishvananda/netlink"
22+
23+
"istio.io/istio/cni/pkg/config"
24+
"istio.io/istio/cni/pkg/ipset"
25+
"istio.io/istio/cni/pkg/util"
26+
)
27+
28+
// detectIptablesArtifacts checks for the presence of Istio iptables artifacts (specifically IPsets)
29+
// on the host network to determine if a previous iptables-based deployment exists.
30+
// Returns:
31+
// - true if iptables artifacts (IPsets) are detected
32+
// - false if no artifacts are detected or if detection fails
33+
// - error if there was a failure during detection
34+
func detectIptablesArtifacts(enableIPv6 bool) (bool, error) {
35+
var detected bool
36+
var detectionErr error
37+
38+
// Run the detection in the host network namespace
39+
err := util.RunAsHost(func() error {
40+
// Check if the IPv4 IPset exists
41+
v4Name := fmt.Sprintf(ipset.V4Name, config.ProbeIPSet)
42+
v4Exists, err := ipsetExists(v4Name)
43+
if err != nil {
44+
log.Debugf("failed to check for v4 IPset %s: %v", v4Name, err)
45+
detectionErr = fmt.Errorf("v4 IPset detection failed: %w", err)
46+
// Lets continue checking for v6 (if enabled)
47+
}
48+
49+
if v4Exists {
50+
log.Infof("detected iptables artifact: IPset %s exists", v4Name)
51+
detected = true
52+
// Found v4 artifact, no need to check for v6
53+
return nil
54+
}
55+
56+
// Check if the IPv6 IPset exists
57+
if enableIPv6 {
58+
v6Name := fmt.Sprintf(ipset.V6Name, config.ProbeIPSet)
59+
v6Exists, err := ipsetExists(v6Name)
60+
if err != nil {
61+
log.Debugf("failed to check for v6 IPset %s: %v", v6Name, err)
62+
if detectionErr != nil {
63+
detectionErr = fmt.Errorf("%w; v6 IPset detection failed: %w", detectionErr, err)
64+
} else {
65+
detectionErr = fmt.Errorf("v6 IPset detection failed: %w", err)
66+
}
67+
}
68+
69+
if v6Exists {
70+
log.Infof("detected iptables artifact: IPset %s exists", v6Name)
71+
detected = true
72+
}
73+
}
74+
75+
return nil
76+
})
77+
if err != nil {
78+
return false, fmt.Errorf("failed to run detection in host namespace: %w", err)
79+
}
80+
81+
return detected, detectionErr
82+
}
83+
84+
// ipsetExists checks if an IPset with the given name exists on the host.
85+
// Returns:
86+
// - true, nil if the IPset exists
87+
// - false, nil if the IPset does not exist (expected for clean/fresh setup)
88+
// - false, error for any errors
89+
func ipsetExists(name string) (bool, error) {
90+
_, err := netlink.IpsetList(name)
91+
if err == nil {
92+
// IPset exists
93+
return true, nil
94+
}
95+
96+
if strings.Contains(err.Error(), "no such file") {
97+
// IPSet does not exist
98+
return false, nil
99+
}
100+
101+
return false, err
102+
}

‎cni/pkg/nodeagent/detect_artifacts_linux_test.go‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
// Copyright Istio Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package nodeagent
16+
17+
import (
18+
"fmt"
19+
"strings"
20+
"sync"
21+
"testing"
22+
23+
// Create a new network namespace. This will have the 'lo' interface ready but nothing else.
24+
_ "github.com/howardjohn/unshare-go/netns"
25+
// Create a new user namespace. This will map the current UID/GID to 0.
26+
"github.com/howardjohn/unshare-go/userns"
27+
"github.com/vishvananda/netlink"
28+
"golang.org/x/sys/unix"
29+
30+
"istio.io/istio/cni/pkg/config"
31+
"istio.io/istio/cni/pkg/ipset"
32+
"istio.io/istio/pkg/test/util/assert"
33+
)
34+
35+
// TestDetectIptablesArtifacts is an e2e test that validates the iptable artifacts on the network.
36+
// It runs in an isolated network namespace created by unshare-go, which provides a clean environment
37+
// with root-like capabilities for netlink operations. It validates the upgrade path from iptables to
38+
// nftables backend by simulating various scenarios.
39+
func TestDetectIptablesArtifacts(t *testing.T) {
40+
setup(t)
41+
42+
tests := []struct {
43+
name string
44+
enableIPv6 bool
45+
setupFunc func(t *testing.T)
46+
cleanupFunc func(t *testing.T)
47+
expectedDetected bool
48+
description string
49+
}{
50+
{
51+
name: "v4_ipset_exists",
52+
enableIPv6: false,
53+
setupFunc: func(t *testing.T) {
54+
v4Name := fmt.Sprintf(ipset.V4Name, config.ProbeIPSet)
55+
err := netlink.IpsetCreate(v4Name, "hash:ip", netlink.IpsetCreateOptions{Replace: true})
56+
if err != nil && !strings.Contains(err.Error(), "exist") {
57+
t.Fatalf("failed to create v4 ipset: %v", err)
58+
}
59+
t.Logf("Created v4 IPset: %s", v4Name)
60+
},
61+
cleanupFunc: func(t *testing.T) {
62+
v4Name := fmt.Sprintf(ipset.V4Name, config.ProbeIPSet)
63+
_ = netlink.IpsetDestroy(v4Name)
64+
},
65+
expectedDetected: true,
66+
description: "Should detect when v4 IPset exists (simulates an existing iptables deployment)",
67+
},
68+
{
69+
name: "v6_ipset_exists",
70+
enableIPv6: true,
71+
setupFunc: func(t *testing.T) {
72+
v6Name := fmt.Sprintf(ipset.V6Name, config.ProbeIPSet)
73+
err := netlink.IpsetCreate(v6Name, "hash:ip", netlink.IpsetCreateOptions{
74+
Family: unix.AF_INET6,
75+
Replace: true,
76+
})
77+
if err != nil && !strings.Contains(err.Error(), "exist") {
78+
t.Fatalf("failed to create v6 ipset: %v", err)
79+
}
80+
t.Logf("Created v6 IPset: %s", v6Name)
81+
},
82+
cleanupFunc: func(t *testing.T) {
83+
v6Name := fmt.Sprintf(ipset.V6Name, config.ProbeIPSet)
84+
_ = netlink.IpsetDestroy(v6Name)
85+
},
86+
expectedDetected: true,
87+
description: "Should detect when v6 IPset exists",
88+
},
89+
{
90+
name: "both_v4_and_v6_ipsets_exist",
91+
enableIPv6: true,
92+
setupFunc: func(t *testing.T) {
93+
v4Name := fmt.Sprintf(ipset.V4Name, config.ProbeIPSet)
94+
v6Name := fmt.Sprintf(ipset.V6Name, config.ProbeIPSet)
95+
96+
err := netlink.IpsetCreate(v4Name, "hash:ip", netlink.IpsetCreateOptions{Replace: true})
97+
if err != nil && !strings.Contains(err.Error(), "exist") {
98+
t.Fatalf("failed to create v4 ipset: %v", err)
99+
}
100+
101+
err = netlink.IpsetCreate(v6Name, "hash:ip", netlink.IpsetCreateOptions{
102+
Family: unix.AF_INET6,
103+
Replace: true,
104+
})
105+
if err != nil && !strings.Contains(err.Error(), "exist") {
106+
t.Fatalf("failed to create v6 ipset: %v", err)
107+
}
108+
t.Logf("Created both v4 and v6 IPsets")
109+
},
110+
cleanupFunc: func(t *testing.T) {
111+
v4Name := fmt.Sprintf(ipset.V4Name, config.ProbeIPSet)
112+
v6Name := fmt.Sprintf(ipset.V6Name, config.ProbeIPSet)
113+
_ = netlink.IpsetDestroy(v4Name)
114+
_ = netlink.IpsetDestroy(v6Name)
115+
},
116+
expectedDetected: true,
117+
description: "Should detect when both v4 and v6 IPsets exist",
118+
},
119+
{
120+
name: "no_ipsets_exist",
121+
enableIPv6: true,
122+
setupFunc: func(t *testing.T) {},
123+
cleanupFunc: func(t *testing.T) {},
124+
expectedDetected: false,
125+
description: "Should not detect artifacts in clean state (simulates a fresh setup)",
126+
},
127+
}
128+
129+
for _, tt := range tests {
130+
t.Run(tt.name, func(t *testing.T) {
131+
t.Log(tt.description)
132+
133+
tt.setupFunc(t)
134+
defer tt.cleanupFunc(t)
135+
136+
detected, _ := detectIptablesArtifacts(tt.enableIPv6)
137+
assert.Equal(t, detected, tt.expectedDetected)
138+
})
139+
}
140+
}
141+
142+
var initialized = &sync.Once{}
143+
144+
// setup initializes the test environment using unshare-go.
145+
// Importing "github.com/howardjohn/unshare-go/netns" causes the test to run in an isolated network
146+
// namespace and userns provides user namespace mapping.
147+
func setup(t *testing.T) {
148+
initialized.Do(func() {
149+
// Map current GID to root (0) in the user namespace
150+
// This gives us the necessary privileges for netlink operations (CAP_NET_ADMIN)
151+
assert.NoError(t, userns.WriteGroupMap(map[uint32]uint32{userns.OriginalGID(): 0}))
152+
t.Log("Successfully initialized an isolated test network namespace with root capabilities")
153+
})
154+
}

‎cni/pkg/nodeagent/server_linux.go‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,32 @@ func initMeshDataplane(client kube.Client, args AmbientArgs) (*meshDataplane, er
4646
Reconcile: args.ReconcilePodRulesOnStartup,
4747
}
4848

49+
useNftables := args.NativeNftables
50+
51+
// To support safe migration from iptables to nftables backend, detect if the host already has any iptable artifacts.
52+
if useNftables {
53+
log.Info("Native nftables is configured, checking for iptables artifacts...")
54+
iptablesDetected, err := detectIptablesArtifacts(args.EnableIPv6)
55+
56+
if iptablesDetected {
57+
// Override nftables configuration and continue with iptables backend
58+
log.Warnf("iptables artifacts detected (IPsets exist). " +
59+
"Overriding nftables configuration and continuing with iptables backend. " +
60+
"To complete migration to nftables, reboot the node.")
61+
useNftables = false
62+
} else {
63+
if err != nil {
64+
// Error while detecting the artifacts. Default to nftables (as requested) for a fail-safe behavior.
65+
log.Warnf("iptables artifacts could not be detected (%v). "+
66+
"Proceeding with nftables backend as configured.", err)
67+
} else {
68+
log.Info("iptables artifacts not found, proceeding with nftables backend")
69+
}
70+
}
71+
}
72+
4973
log.Infof("creating host addressSet manager in the node netns")
50-
setManager, err := createHostNetworkAddrSetManager(args.NativeNftables, hostCfg.EnableIPv6)
74+
setManager, err := createHostNetworkAddrSetManager(useNftables, hostCfg.EnableIPv6)
5175
if err != nil {
5276
return nil, fmt.Errorf("error initializing host addressSet manager: %w", err)
5377
}
@@ -59,7 +83,7 @@ func initMeshDataplane(client kube.Client, args AmbientArgs) (*meshDataplane, er
5983
}
6084

6185
hostTrafficManager, podTrafficManager, err := trafficmanager.NewTrafficRuleManager(&trafficmanager.TrafficRuleManagerConfig{
62-
NativeNftables: args.NativeNftables,
86+
NativeNftables: useNftables,
6387
HostConfig: hostCfg,
6488
PodConfig: podCfg,
6589
HostDeps: realDependenciesHost(args.ForceIptablesBinary),
@@ -70,7 +94,7 @@ func initMeshDataplane(client kube.Client, args AmbientArgs) (*meshDataplane, er
7094
return nil, fmt.Errorf("error creating traffic managers: %w", err)
7195
}
7296

73-
if !args.NativeNftables {
97+
if !useNftables {
7498
// The nftables implementation will automatically flush any pre-existing chains when programming
7599
// the rules, so we skip the DeleteHostRules for nftables backend.
76100
hostTrafficManager.DeleteHostRules()

‎releasenotes/notes/58353.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
apiVersion: release-notes/v2
2+
kind: bug-fix
3+
area: traffic-management
4+
issue:
5+
- 58353
6+
releaseNotes:
7+
- |
8+
**Fixed** use cases where upgrading from the iptables backend to the nftables backend in ambient created stale iptables rules on the network. The code now continues to use iptables on the node until it is rebooted.

0 commit comments

Comments
 (0)