Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Revert agent/helm: Deprecate --kpr=partial|strict|disabled and use --kpr=true|false instead" #26496

Merged
merged 5 commits into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
36 changes: 18 additions & 18 deletions .github/workflows/conformance-e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -180,26 +180,26 @@ jobs:
- name: '1'
kernel: '4.19-20230420.212204'
kube-proxy: 'iptables'
kpr: 'disabled'
kpr: 'false'
tunnel: 'vxlan'

- name: '2'
kernel: '5.4-20230420.212204'
kube-proxy: 'iptables'
kpr: 'disabled'
kpr: 'false'
tunnel: 'disabled'

- name: '3'
kernel: '5.10-20230420.212204'
kube-proxy: 'iptables'
kpr: 'disabled'
kpr: 'false'
tunnel: 'disabled'
endpoint-routes: 'true'

- name: '4'
kernel: '5.10-20230420.212204'
kube-proxy: 'iptables'
kpr: 'strict'
kpr: 'true'
tunnel: 'vxlan'
lb-mode: 'snat'
endpoint-routes: 'true'
Expand All @@ -208,7 +208,7 @@ jobs:
- name: '5'
kernel: '5.15-20230420.212204'
kube-proxy: 'iptables'
kpr: 'strict'
kpr: 'true'
tunnel: 'disabled'
lb-mode: 'dsr'
endpoint-routes: 'true'
Expand All @@ -218,7 +218,7 @@ jobs:
- name: '6'
kernel: '6.0-20230420.212204'
kube-proxy: 'none'
kpr: 'strict'
kpr: 'true'
tunnel: 'vxlan'
lb-mode: 'snat'
egress-gateway: 'true'
Expand All @@ -228,7 +228,7 @@ jobs:
- name: '7'
kernel: 'bpf-next-20230420.212204'
kube-proxy: 'none'
kpr: 'strict'
kpr: 'true'
tunnel: 'disabled'
lb-mode: 'snat'
egress-gateway: 'true'
Expand All @@ -237,30 +237,30 @@ jobs:
- name: '8'
kernel: 'bpf-next-20230420.212204'
kube-proxy: 'iptables'
kpr: 'disabled'
kpr: 'false'
tunnel: 'geneve'
endpoint-routes: 'true'

- name: '9'
kernel: '4.19-20230420.212204'
kube-proxy: 'iptables'
kpr: 'disabled'
kpr: 'false'
tunnel: 'vxlan'
encryption: 'ipsec'
encryption-node: 'false'

- name: '10'
kernel: '5.4-20230420.212204'
kube-proxy: 'iptables'
kpr: 'disabled'
kpr: 'false'
tunnel: 'disabled'
encryption: 'ipsec'
encryption-node: 'false'

- name: '11'
kernel: '5.10-20230420.212204'
kube-proxy: 'iptables'
kpr: 'disabled'
kpr: 'false'
tunnel: 'disabled'
encryption: 'ipsec'
encryption-node: 'false'
Expand All @@ -269,7 +269,7 @@ jobs:
- name: '12'
kernel: '5.10-20230420.212204'
kube-proxy: 'iptables'
kpr: 'strict'
kpr: 'true'
tunnel: 'vxlan'
encryption: 'wireguard'
encryption-node: 'false'
Expand All @@ -280,7 +280,7 @@ jobs:
- name: '13'
kernel: '5.15-20230420.212204'
kube-proxy: 'iptables'
kpr: 'strict'
kpr: 'true'
tunnel: 'disabled'
encryption: 'wireguard'
encryption-node: 'false'
Expand All @@ -291,7 +291,7 @@ jobs:
- name: '14'
kernel: '6.0-20230420.212204'
kube-proxy: 'none'
kpr: 'strict'
kpr: 'true'
tunnel: 'vxlan'
encryption: 'wireguard'
encryption-node: 'true'
Expand All @@ -301,7 +301,7 @@ jobs:
- name: '15'
kernel: 'bpf-next-20230420.212204'
kube-proxy: 'none'
kpr: 'strict'
kpr: 'true'
tunnel: 'disabled'
encryption: 'wireguard'
encryption-node: 'true'
Expand All @@ -311,7 +311,7 @@ jobs:
- name: '16'
kernel: 'bpf-next-20230420.212204'
kube-proxy: 'iptables'
kpr: 'disabled'
kpr: 'false'
tunnel: 'geneve'
encryption: 'ipsec'
encryption-node: 'false'
Expand Down Expand Up @@ -366,8 +366,8 @@ jobs:
IPV6="--helm-set=ipv6.enabled=true"
fi
MASQ=""
if [ "${{ matrix.kpr }}" == "strict" ]; then
# BPF-masq requires KPR=strict.
if [ "${{ matrix.kpr }}" == "true" ]; then
# BPF-masq requires KPR=true.
MASQ="--helm-set=bpf.masquerade=true"
if [ "${{ matrix.host-fw }}" == "true" ]; then
# BPF IPv6 masquerade not currently supported with host firewall - GH-26074
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/conformance-gateway-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ jobs:
--helm-set=operator.image.suffix=-ci \
--helm-set=operator.image.tag=${SHA} \
--helm-set=operator.image.useDigest=false \
--helm-set kubeProxyReplacement=strict \
--helm-set kubeProxyReplacement=true \
--helm-set=securityContext.privileged=true \
--helm-set=gatewayAPI.enabled=true"

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/conformance-ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ jobs:
--helm-set=operator.image.tag=${SHA} \
--helm-set=operator.image.useDigest=false \
--helm-set=securityContext.privileged=true \
--helm-set kubeProxyReplacement=strict \
--helm-set kubeProxyReplacement=true \
--helm-set=ingressController.enabled=true \
--helm-set=ingressController.loadbalancerMode=${{ matrix.loadbalancer-mode }} \
--helm-set=ingressController.default=${{ matrix.default-ingress-controller }} \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ jobs:
--helm-set=hubble.relay.image.repository=quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/hubble-relay-ci \
--helm-set=hubble.relay.image.tag=${SHA} \
--helm-set=cni.chainingMode=portmap \
--helm-set-string=kubeProxyReplacement=strict \
--helm-set=kubeProxyReplacement=true \
--helm-set=sessionAffinity=true \
--helm-set=identityChangeGracePeriod="0s" \
--rollback=false \
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests-smoke-ipv6.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ jobs:
run: |
CILIUM_INSTALL_DEFAULTS="--chart-directory=install/kubernetes/cilium \
--helm-set nodeinit.enabled=true \
--helm-set kubeProxyReplacement=strict \
--helm-set kubeProxyReplacement=true \
--helm-set ipam.mode=kubernetes \
--helm-set image.repository=quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/cilium-ci \
--helm-set image.tag=${{ steps.sha.outputs.tag }} \
Expand Down
7 changes: 1 addition & 6 deletions .github/workflows/tests-smoke.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,7 @@ jobs:
run: |
CILIUM_INSTALL_DEFAULTS="--chart-directory=install/kubernetes/cilium \
--helm-set nodeinit.enabled=true \
--helm-set kubeProxyReplacement=partial \
--helm-set socketLB.enabled=false \
--helm-set externalIPs.enabled=true \
--helm-set nodePort.enabled=true \
--helm-set hostPort.enabled=true \
--helm-set bpf.masquerade=false \
--helm-set kubeProxyReplacement=true \
--helm-set ipam.mode=kubernetes \
--helm-set image.repository=quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/cilium-ci \
--helm-set image.tag=${{ steps.sha.outputs.tag }} \
Expand Down
6 changes: 3 additions & 3 deletions Documentation/cmdref/cilium-agent.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions api/v1/models/kube_proxy_replacement.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions api/v1/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2076,6 +2076,8 @@ definitions:
- Strict
- Probe
- Partial
- 'True'
- 'False'
devices:
type: array
items:
Expand Down
8 changes: 6 additions & 2 deletions api/v1/server/embedded_spec.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 6 additions & 8 deletions daemon/cmd/daemon_main.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func initializeFlags() {
flags.String(option.EnablePolicy, option.DefaultEnforcement, "Enable policy enforcement")
option.BindEnv(Vp, option.EnablePolicy)

flags.Bool(option.EnableExternalIPs, defaults.EnableExternalIPs, fmt.Sprintf("Enable k8s service externalIPs feature (requires enabling %s)", option.EnableNodePort))
flags.Bool(option.EnableExternalIPs, false, fmt.Sprintf("Enable k8s service externalIPs feature (requires enabling %s)", option.EnableNodePort))
option.BindEnv(Vp, option.EnableExternalIPs)

flags.Bool(option.K8sEnableEndpointSlice, defaults.K8sEnableEndpointSlice, "Enables k8s EndpointSlice feature in Cilium if the k8s cluster supports it")
Expand Down Expand Up @@ -523,18 +523,16 @@ func initializeFlags() {
flags.StringSlice(option.Labels, []string{}, "List of label prefixes used to determine identity of an endpoint")
option.BindEnv(Vp, option.Labels)

flags.String(option.KubeProxyReplacement, option.KubeProxyReplacementPartial, fmt.Sprintf(
flags.String(option.KubeProxyReplacement, option.KubeProxyReplacementFalse, fmt.Sprintf(
"Enable only selected features (will panic if any selected feature cannot be enabled) (%q), "+
"or enable all features (will panic if any feature cannot be enabled) (%q), "+
"or completely disable it (ignores any selected feature) (%q)",
option.KubeProxyReplacementPartial, option.KubeProxyReplacementStrict,
option.KubeProxyReplacementDisabled))
"or enable all features (will panic if any feature cannot be enabled) (%q)",
option.KubeProxyReplacementFalse, option.KubeProxyReplacementTrue))
option.BindEnv(Vp, option.KubeProxyReplacement)

flags.String(option.KubeProxyReplacementHealthzBindAddr, defaults.KubeProxyReplacementHealthzBindAddr, "The IP address with port for kube-proxy replacement health check server to serve on (set to '0.0.0.0:10256' for all IPv4 interfaces and '[::]:10256' for all IPv6 interfaces). Set empty to disable.")
option.BindEnv(Vp, option.KubeProxyReplacementHealthzBindAddr)

flags.Bool(option.EnableHostPort, true, fmt.Sprintf("Enable k8s hostPort mapping feature (requires enabling %s)", option.EnableNodePort))
flags.Bool(option.EnableHostPort, false, fmt.Sprintf("Enable k8s hostPort mapping feature (requires enabling %s)", option.EnableNodePort))
option.BindEnv(Vp, option.EnableHostPort)

flags.Bool(option.EnableNodePort, false, "Enable NodePort type services by Cilium")
Expand Down Expand Up @@ -1328,7 +1326,7 @@ func initEnv() {
if option.Config.NodePortAcceleration != option.NodePortAccelerationDisabled {
option.Config.EnablePMTUDiscovery = true
}
option.Config.KubeProxyReplacement = option.KubeProxyReplacementPartial
option.Config.KubeProxyReplacement = option.KubeProxyReplacementFalse
option.Config.EnableSocketLB = true
// Socket-LB tracing relies on metadata that's retrieved from Kubernetes.
option.Config.EnableSocketLBTracing = false
Expand Down
2 changes: 1 addition & 1 deletion daemon/cmd/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestMain(m *testing.M) {
// Disable the replacement, as its initialization function execs bpftool
// which requires root privileges. This would require marking the test suite
// as privileged.
option.Config.KubeProxyReplacement = option.KubeProxyReplacementDisabled
option.Config.KubeProxyReplacement = option.KubeProxyReplacementFalse

time.Local = time.UTC

Expand Down
21 changes: 16 additions & 5 deletions daemon/cmd/kube_proxy_replacement.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,19 @@ import (
func initKubeProxyReplacementOptions() error {
if option.Config.KubeProxyReplacement != option.KubeProxyReplacementStrict &&
option.Config.KubeProxyReplacement != option.KubeProxyReplacementPartial &&
option.Config.KubeProxyReplacement != option.KubeProxyReplacementDisabled {
option.Config.KubeProxyReplacement != option.KubeProxyReplacementDisabled &&
option.Config.KubeProxyReplacement != option.KubeProxyReplacementTrue &&
option.Config.KubeProxyReplacement != option.KubeProxyReplacementFalse {
return fmt.Errorf("Invalid value for --%s: %s", option.KubeProxyReplacement, option.Config.KubeProxyReplacement)
}

if option.Config.KubeProxyReplacement == option.KubeProxyReplacementStrict ||
option.Config.KubeProxyReplacement == option.KubeProxyReplacementPartial ||
option.Config.KubeProxyReplacement == option.KubeProxyReplacementDisabled {
log.Warnf("Deprecated value for --%s: %s (use either \"true\", or \"false\")", option.KubeProxyReplacement, option.Config.KubeProxyReplacement)
pchaigno marked this conversation as resolved.
Show resolved Hide resolved
}

// This will be removed in v1.15
if option.Config.KubeProxyReplacement == option.KubeProxyReplacementDisabled {
log.Infof("Auto-disabling %q, %q, %q, %q features and falling back to %q",
option.EnableNodePort, option.EnableExternalIPs,
Expand All @@ -62,7 +71,9 @@ func initKubeProxyReplacementOptions() error {
return nil
}

if option.Config.KubeProxyReplacement == option.KubeProxyReplacementStrict {
if option.Config.KubeProxyReplacement == option.KubeProxyReplacementStrict ||
option.Config.KubeProxyReplacement == option.KubeProxyReplacementTrue {

log.Infof("Auto-enabling %q, %q, %q, %q, %q features",
option.EnableNodePort, option.EnableExternalIPs,
option.EnableSocketLB, option.EnableHostPort,
Expand Down Expand Up @@ -242,7 +253,7 @@ func initKubeProxyReplacementOptions() error {
// required for NAT operations
if !option.Config.KubeProxyReplacementFullyEnabled() {
return fmt.Errorf("%s requires the agent to run with %s=%s.",
option.InstallNoConntrackIptRules, option.KubeProxyReplacement, option.KubeProxyReplacementStrict)
option.InstallNoConntrackIptRules, option.KubeProxyReplacement, option.KubeProxyReplacementTrue)
}

if option.Config.MasqueradingEnabled() && !option.Config.EnableBPFMasquerade {
Expand Down Expand Up @@ -413,8 +424,8 @@ func finishKubeProxyReplacementInit() error {
case option.Config.IptablesMasqueradingEnabled():
msg = fmt.Sprintf("BPF host routing requires %s.", option.EnableBPFMasquerade)
// KPR=strict is needed or we might rely on netfilter.
case option.Config.KubeProxyReplacement != option.KubeProxyReplacementStrict:
msg = fmt.Sprintf("BPF host routing requires %s=%s.", option.KubeProxyReplacement, option.KubeProxyReplacementStrict)
case option.Config.KubeProxyReplacement != option.KubeProxyReplacementStrict && option.Config.KubeProxyReplacement != option.KubeProxyReplacementTrue:
msg = fmt.Sprintf("BPF host routing requires %s=%s.", option.KubeProxyReplacement, option.KubeProxyReplacementTrue)
default:
if probes.HaveProgramHelper(ebpf.SchedCLS, asm.FnRedirectNeigh) != nil ||
probes.HaveProgramHelper(ebpf.SchedCLS, asm.FnRedirectPeer) != nil {
Expand Down