doc: Ensure ConfigMap remains compatible across 1.7 -> 1.8 upgrade #12097

tgraf · 2020-06-16T12:04:25Z

No description provided.

maintainer-s-little-helper · 2020-06-16T12:04:27Z

Please set the appropriate release note label.

coveralls · 2020-06-16T12:53:41Z

Coverage decreased (-0.01%) to 37.06% when pulling 3886ce5 on pr/tgraf/fix-upgrade-compatibility into 989eced on master.

tgraf · 2020-06-16T13:23:57Z

test-me-please

Documentation/install/upgrade.rst

install/kubernetes/cilium/charts/config/templates/configmap.yaml

joestringer

Shouldn't we be updating the K8sUpdates test to use this new method?

joestringer · 2020-06-16T19:51:34Z

Some of the failures look likely related to disabling CNP updates:

Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-newest-kernel-4.9/k8s-1.18-gopath/src/github.com/cilium/cilium/test/k8sT/manifests/l7-policy-demo.yaml: Timed out while waiting CNP to be enforced: 4m0s timeout expired
Expected
    <*errors.errorString | 0xc000566390>: {
        s: "Timed out while waiting CNP to be enforced: 4m0s timeout expired",
    }
to be nil

tgraf · 2020-06-16T20:07:42Z

Some of the failures look likely related to disabling CNP updates:

This construct still doesn't work:

    enable-session-affinity: {{ default $defaultSessionAffinity .Values.sessionAffinity | quote }}

I think we have to write it as:

  {{- if hasKey .Values "sessionAffinity" }}
    enable-session-affinity: {{ .Values.sessionAffinity | quote }}
  {{- else if eq $defaultSessionAffinity "true" }}
    enable-session-affinity: {{ $defaultSessionAffinity | quote }}
  {{- end }}

default true $var will always result in true, regardless of whether $var is true, false, or empty/unset.

tgraf · 2020-06-16T20:36:33Z

Shouldn't we be updating the K8sUpdates test to use this new method?

That's a fantastic idea!

tgraf · 2020-06-17T09:04:13Z

test-me-please

install/kubernetes/cilium/charts/config/templates/configmap.yaml

tgraf · 2020-06-17T12:51:07Z

test-me-please

tgraf · 2020-06-17T13:53:33Z

Will rebase after tests have passed

When running with option `--set upgradeCompatibility=1.7`, then the diff between the ConfigMaps is: ``` @@ -60,8 +60,7 @@ # # Only effective when monitor aggregation is set to "medium" or higher. monitor-aggregation-flags: all - - # ct-global-max-entries-* specifies the maximum number of connections + # bpf-ct-global-*-max specifies the maximum number of connections # supported across all endpoints, split by protocol: tcp or other. One pair # of maps uses these values for IPv4 connections, and another pair of maps # use these values for IPv6 connections. @@ -71,10 +70,9 @@ # policy drops or a change in loadbalancing decisions for a connection. # # For users upgrading from Cilium 1.2 or earlier, to minimize disruption - # during the upgrade process, comment out these options. + # during the upgrade process, set bpf-ct-global-tcp-max to 1000000. bpf-ct-global-tcp-max: "524288" bpf-ct-global-any-max: "262144" - # bpf-policy-map-max specified the maximum number of entries in endpoint # policy map (per endpoint) bpf-policy-map-max: "16384" @@ -140,9 +138,6 @@ install-iptables-rules: "true" auto-direct-node-routes: "false" kube-proxy-replacement: "probe" - enable-host-reachable-services: "false" - enable-external-ips: "false" - enable-node-port: "false" node-port-bind-protection: "true" enable-auto-protect-node-port-range: "true" enable-endpoint-health-checking: "true" ``` When running without upgradeCompatibility, the diff is: ``` @@ -43,6 +43,7 @@ # Enable IPv6 addressing. If enabled, all endpoints are allocated an IPv6 # address. enable-ipv6: "false" + enable-bpf-clock-probe: "true" # If you want cilium monitor to aggregate tracing for packets, set this level # to "low", "medium", or "maximum". The higher the level, the less packets @@ -60,24 +61,12 @@ # # Only effective when monitor aggregation is set to "medium" or higher. monitor-aggregation-flags: all - - # ct-global-max-entries-* specifies the maximum number of connections - # supported across all endpoints, split by protocol: tcp or other. One pair - # of maps uses these values for IPv4 connections, and another pair of maps - # use these values for IPv6 connections. - # - # If these values are modified, then during the next Cilium startup the - # tracking of ongoing connections may be disrupted. This may lead to brief - # policy drops or a change in loadbalancing decisions for a connection. - # - # For users upgrading from Cilium 1.2 or earlier, to minimize disruption - # during the upgrade process, comment out these options. - bpf-ct-global-tcp-max: "524288" - bpf-ct-global-any-max: "262144" - # bpf-policy-map-max specified the maximum number of entries in endpoint # policy map (per endpoint) bpf-policy-map-max: "16384" + # Specifies the ratio (0.0-1.0) of total system memory to use for dynamic + # sizing of the TCP CT, non-TCP CT, NAT and policy BPF maps. + bpf-map-dynamic-size-ratio: "0.0025" # Pre-allocation of map entries allows per-packet latency to be reduced, at # the expense of up-front memory allocation for the entries in the maps. The @@ -136,18 +125,24 @@ wait-bpf-mount: "false" masquerade: "true" + enable-bpf-masquerade: "true" enable-xt-socket-fallback: "true" install-iptables-rules: "true" auto-direct-node-routes: "false" kube-proxy-replacement: "probe" - enable-host-reachable-services: "false" - enable-external-ips: "false" - enable-node-port: "false" node-port-bind-protection: "true" enable-auto-protect-node-port-range: "true" + enable-session-affinity: "true" + k8s-require-ipv4-pod-cidr: "true" + k8s-require-ipv6-pod-cidr: "false" enable-endpoint-health-checking: "true" enable-well-known-identities: "false" enable-remote-node-identity: "true" + operator-api-serve-addr: "127.0.0.1:9234" + ipam: "cluster-pool" + cluster-pool-ipv4-cidr: "10.0.0.0/8" + cluster-pool-ipv4-mask-size: "24" + disable-cnp-status-updates: "true" ``` Signed-off-by: Thomas Graf <thomas@cilium.io>

tgraf · 2020-06-17T17:46:53Z

test-me-please

tgraf · 2020-06-17T17:48:52Z

test-me-please

tgraf · 2020-06-17T19:33:45Z

Single test failed:

Suite-k8s-1.17.K8sServicesTest Bookinfo Demo Tests bookinfo demo

This is a completely new test failure so it is likely related to the PR.

christarazi · 2020-06-17T22:58:00Z

It looks it hit a newly discovered flake: #12130

It is passing locally for me.

tgraf · 2020-06-17T23:07:59Z

retest-4.19

tgraf requested review from a team as code owners June 16, 2020 12:04

tgraf requested a review from a team June 16, 2020 12:04

maintainer-s-little-helper bot added the dont-merge/needs-release-note label Jun 16, 2020

maintainer-s-little-helper bot added this to In progress in 1.8.0 Jun 16, 2020

tgraf marked this pull request as draft June 16, 2020 12:04

tgraf added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. labels Jun 16, 2020

maintainer-s-little-helper bot removed the dont-merge/needs-release-note label Jun 16, 2020

tgraf force-pushed the pr/tgraf/fix-upgrade-compatibility branch from 008f502 to 805a16f Compare June 16, 2020 12:57

tgraf added the priority/release-blocker label Jun 16, 2020

tgraf force-pushed the pr/tgraf/fix-upgrade-compatibility branch 2 times, most recently from 10827bd to 4d354ea Compare June 16, 2020 13:06

tgraf marked this pull request as ready for review June 16, 2020 13:24

aanm requested changes Jun 16, 2020

View reviewed changes

Documentation/install/upgrade.rst Show resolved Hide resolved

Documentation/install/upgrade.rst Show resolved Hide resolved

errordeveloper reviewed Jun 16, 2020

View reviewed changes

Documentation/install/upgrade.rst Outdated Show resolved Hide resolved

joestringer approved these changes Jun 16, 2020

View reviewed changes

install/kubernetes/cilium/charts/config/templates/configmap.yaml Show resolved Hide resolved

joestringer requested changes Jun 16, 2020

View reviewed changes

tgraf force-pushed the pr/tgraf/fix-upgrade-compatibility branch 2 times, most recently from 5f78ae9 to 282d1df Compare June 16, 2020 20:23

tgraf requested a review from aanm June 16, 2020 20:25

tgraf requested a review from joestringer June 16, 2020 20:38

tgraf force-pushed the pr/tgraf/fix-upgrade-compatibility branch from 282d1df to 4d6d8b3 Compare June 16, 2020 20:38

brb mentioned this pull request Jun 17, 2020

CI: Number of ready Cilium pods: 2, Only 2/3 nodes appeared in cilium-health status. nodes = '[k8s1 k8s2 k8s3]' #12128

Closed

brb reviewed Jun 17, 2020

View reviewed changes

install/kubernetes/cilium/charts/config/templates/configmap.yaml Outdated Show resolved Hide resolved

tgraf force-pushed the pr/tgraf/fix-upgrade-compatibility branch from 421d240 to f692c56 Compare June 17, 2020 12:35

tgraf added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 17, 2020

tgraf force-pushed the pr/tgraf/fix-upgrade-compatibility branch from f692c56 to 3886ce5 Compare June 17, 2020 17:46

joestringer approved these changes Jun 17, 2020

View reviewed changes

tgraf merged commit 8ec15e2 into master Jun 18, 2020

1.8.0 automation moved this from In progress to Merged Jun 18, 2020

tgraf deleted the pr/tgraf/fix-upgrade-compatibility branch June 18, 2020 07:04

tgraf added needs-backport/1.8 and removed dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. labels Jun 18, 2020

maintainer-s-little-helper bot added this to Needs backport from master in 1.8.0 Jun 18, 2020

jrajahalme mentioned this pull request Jun 18, 2020

v1.8 backports 2020-06-18 #12173

Merged

jrajahalme added backport-pending/1.8 and removed needs-backport/1.8 labels Jun 18, 2020

maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.0 Jun 18, 2020

aanm mentioned this pull request Jun 18, 2020

operator: Make CRD availability timeout configurable #12177

Merged

borkmann added backport-done/1.8 and removed backport-pending/1.8 labels Jun 18, 2020

maintainer-s-little-helper bot moved this from Backport pending to v1.8 to Backport done to v1.8 in 1.8.0 Jun 18, 2020

aanm mentioned this pull request Jun 25, 2020

[RFC] install/kubernetes: provide way to keep ConfigMap for upgrades #12080

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: Ensure ConfigMap remains compatible across 1.7 -> 1.8 upgrade #12097

doc: Ensure ConfigMap remains compatible across 1.7 -> 1.8 upgrade #12097

tgraf commented Jun 16, 2020

maintainer-s-little-helper bot commented Jun 16, 2020

coveralls commented Jun 16, 2020 •

edited

tgraf commented Jun 16, 2020

joestringer left a comment

joestringer commented Jun 16, 2020

tgraf commented Jun 16, 2020

tgraf commented Jun 16, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020 •

edited

christarazi commented Jun 17, 2020

tgraf commented Jun 17, 2020

doc: Ensure ConfigMap remains compatible across 1.7 -> 1.8 upgrade #12097

doc: Ensure ConfigMap remains compatible across 1.7 -> 1.8 upgrade #12097

Conversation

tgraf commented Jun 16, 2020

maintainer-s-little-helper bot commented Jun 16, 2020

coveralls commented Jun 16, 2020 • edited

tgraf commented Jun 16, 2020

joestringer left a comment

Choose a reason for hiding this comment

joestringer commented Jun 16, 2020

tgraf commented Jun 16, 2020

tgraf commented Jun 16, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020

tgraf commented Jun 17, 2020 • edited

christarazi commented Jun 17, 2020

tgraf commented Jun 17, 2020

coveralls commented Jun 16, 2020 •

edited

tgraf commented Jun 17, 2020 •

edited