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

v1.11 backports 2022-02-17 #18836

Merged
merged 3 commits into from
Feb 21, 2022

Conversation

nebril
Copy link
Member

@nebril nebril commented Feb 17, 2022

Once this PR is merged, you can update the PR labels via:

$ for pr in 18762 18790; do contrib/backporting/set-labels.py $pr done 1.11; done

or with

$ make add-label branch=v1.11 issues=18762,18790

jaffcheng and others added 3 commits February 17, 2022 12:55
[ upstream commit 76e3aac ]

error message:

panic: descriptor Desc{fqName: "cilium_operator_alibaba-cloud_api_duration_seconds", help:
"Duration of interactions with API", constLabels: {}, variableLabels: [operation response_code]} is invalid:
"cilium_operator_alibaba-cloud_api_duration_seconds" is not a valid metric name

Signed-off-by: Jaff Cheng <jaff.cheng.sh@gmail.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
[ upstream commit 842f6c8 ]

Currently, cilium-agent using alibaba ipam mode doesn't
respect pre-allocate configuration from CNI config file when
creating ciliumnode resource, and the value of pre-allocate
is always the default value 8.

This patch makes this option configurable via CNI config.

Signed-off-by: Jaff Cheng <jaff.cheng.sh@gmail.com>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
[ upstream commit 98ca102 ]

We currently have a race condition between functions AddToNodeIpset and
InstallRules (see stacktrace below). AddToNodeIpset creates the ipset
then adds the given IP address to that ipset. At the same time,
InstallRules renames ipsets to a backup name, creates new ipsets, and
removes the backups. Depending on timings, AddToNodeIpset may therefore
attempt to add IPs to a nonexistent ipset.

    runDaemon()
    - NewDameon()
      - InitK8sSubsystem()
        - EnableK8sWatcher()
          * ciliumNodeInit()
            - NodeUpdated()
              - iptables.AddToNodeIpset()
      [...]
      - d.init()
        - d.Datapath().Loader().Reinitialize()
          - InstallRules()
            - removeRulesAndIpsets()

We however don't need InstallRules to use a whole backup system for
ipsets. This backup system makes sense for iptables rules because we may
need to change them based on the agent configuration, but that's not the
case for ipsets; their content doesn't depend on configuration. So
either we need them and should create them, or we don't need them and we
can remove any leftover ipsets. We never need to reset them.

Fixes: 76551df ("iptables: Remove old ipsets")
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril nebril requested a review from a team as a code owner February 17, 2022 11:56
@nebril nebril requested a review from pchaigno February 17, 2022 11:56
@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.11 This PR represents a backport for Cilium 1.11.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. labels Feb 17, 2022
@nebril
Copy link
Member Author

nebril commented Feb 17, 2022

/test-backport-1.11

Job 'Cilium-PR-K8s-1.21-kernel-4.9' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sServicesTest Checks service across nodes with L7 policy Tests NodePort with L7 Policy

Failure Output

FAIL: Request from k8s1 to service http://10.109.210.129:10080 failed

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.21-kernel-4.9 so I can create a new GitHub issue to track it.

Job 'Cilium-PR-K8s-1.23-kernel-4.9' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sServicesTest Checks service across nodes Tests NodePort (kube-proxy) 

Failure Output

FAIL: Request from testclient-kw4c5 pod to service tftp://[fd04::12]:32303/hello failed

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.23-kernel-4.9 so I can create a new GitHub issue to track it.

Job 'Cilium-PR-K8s-1.23-kernel-4.9' failed:

Click to show.

Test Name

K8sPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.23-kernel-4.9 so I can create one.

Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My changes look good. Thanks Maciej!

@pchaigno pchaigno requested a review from brb February 18, 2022 10:47
@nebril
Copy link
Member Author

nebril commented Feb 21, 2022

/ci-aks-1.11

@nebril
Copy link
Member Author

nebril commented Feb 21, 2022

/test-1.21-4.9

@nebril
Copy link
Member Author

nebril commented Feb 21, 2022

/test-1.21-5.4

@nebril
Copy link
Member Author

nebril commented Feb 21, 2022

/test-1.23-4.9

@joestringer
Copy link
Member

k8s-1.23-kernel-4.9 hit #13071, safe to ignore. It's the only failure, merging.

@joestringer joestringer merged commit 9f3c716 into cilium:v1.11 Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.11 This PR represents a backport for Cilium 1.11.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants