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.

Sign up for GitHub

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

Jump to bottom

v1.9 backports 2021-02-11 #14930

Merged
merged 2 commits into from
Feb 12, 2021
Merged

v1.9 backports 2021-02-11 #14930

merged 2 commits into from
Feb 12, 2021

Conversation

michi-covalent
Copy link
Contributor

@michi-covalent michi-covalent commented Feb 11, 2021 •
edited

Skipped:

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

$ for pr in 14114 14893; do contrib/backporting/set-labels.py $pr done 1.9; done

ArthurChiao and others added 2 commits February 11, 2021 04:25
@ArthurChiao @michi-covalent
labelsfilter: add 'reserved:.*' to default label list
6d84d94 
[ upstream commit 16e8f2f ]

Fix #14100

Identity relevant labels is a label prefix list combined of two parts:

1. base part:
  1.1. Read from a user specified (--label-prefix-file) json file if this
      file is provided. Default: `--label-prefix-file=""`.
  1.2 If `--label-prefix-file=""`, read from a default hardcoded list
      (`func defaultLabelPrefixCfg()`).
2. additional part: read from user inputs (--labels), default `--labels=""`

When `--label-prefix-file=""` (default) but `--labels=<custom-list>` provided,
if `reserved:host` (or `reserved:.*`) is not included in the above
`<custom-list>`, the `cilium_host` endpoint will lose its `reserved:host`
label.

When rolling back to the default configuration, that is, setting `--labels=""`
and restarting the agent, cilium agent will raise errors like following:

```
level=warning msg="Regeneration of endpoint failed" .. error="Exposing new BPF failed: invalid LXC MAC: invalid MAC address "
level=error msg="endpoint regeneration failed" ..  error="Exposing new BPF failed: invalid LXC MAC: invalid MAC address "
```

And subsequently, all pods' traffic on this node will be interrupted.

This is because the agent relies on this label to distinguish `cilium_host`
endpoint from normal endpoints, and the former has no `lxcMAC`.
We should never exclude reserved labels from default label list.
Add reserved labels to the default label list could solve the problem.

Appendix:

Sample custom label file (--label-prefix-file) to overwrite the default base
label list:

```
{
    "version": 1,
    "valid-prefixes": [
    {
            "source": "k8s",
            "prefix": "io.kubernetes.pod.namespace"
    }, {
            "source": "k8s",
          "prefix": ":io.cilium.k8s.namespace.labels"
    }, {
            "source": "k8s",
            "prefix": "app.kubernetes.io"
    },{
            "source": "k8s",
            "prefix": "k8s!:io.kubernetes"
    },{
            "source": "k8s",
            "prefix": "!kubernetes.io"
    },{
            "source": "k8s",
            "prefix": "!.*beta.kubernetes.io"
    },{
            "source": "k8s",
            "prefix": "!k8s.io"
    },{
            "source": "k8s",
            "prefix": "!pod-template-generation"
    },{
            "source": "k8s",
            "prefix": "!pod-template-hash"
    },{
            "source": "k8s",
            "prefix": "!controller-revision-hash"
    },{
            "source": "k8s",
            "prefix": "!annotation.*"
    },{
            "source": "k8s",
            "prefix": "!etcd_node"
    ]
}
```

Signed-off-by: ArthurChiao <arthurchiao@hotmail.com>
Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@joestringer @michi-covalent
docs: Add FQDN limitation to IPVLAN docs
e8a6f31 
[ upstream commit 463e0dc ]

Signed-off-by: Joe Stringer <joe@cilium.io>
Signed-off-by: Michi Mutsuzaki <michi@isovalent.com>
@michi-covalent michi-covalent requested a review from a team as a code owner February 11, 2021 04:38
@michi-covalent michi-covalent added backport/1.9 kind/backports This PR provides functionality previously merged into master. labels Feb 11, 2021
@michi-covalent
Copy link
Contributor Author

test-backport-1.9

pchaigno
pchaigno approved these changes Feb 11, 2021
View reviewed changes
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.

I reviewed both commits and they look good to me 👍

@joestringer
Copy link
Member

joestringer commented Feb 11, 2021 •
edited

@michi-covalent when skipping some PRs, don't forget to update the command at the end of the PR description. At the moment it will try to set labels for both the backported and skipped PRs, which might lead to us thinking we've backported those PRs when we actually haven't.

@michi-covalent
Copy link
Contributor Author

@michi-covalent when skipping some PRs, don't forget to update the command at the end of the PR description. At the moment it will try to set labels for both the backported and skipped PRs, which might lead to us thinking we've backported those PRs when we actually haven't.

@joestringer ah yes of course, updated the description 👍

@michi-covalent
Copy link
Contributor Author

test-missed-k8s

This was referenced Feb 12, 2021
Closed
Closed
@pchaigno
Copy link
Member

pchaigno commented Feb 12, 2021 •
edited

K8s-1.15-kernel-4.9 and K8s-1.17-kernel-4.9 failed with known flakes #13773 and #13774. K8s-1.14-kernel-4.9 failed with:

Istio sidecar proxies are not reachable

https://jenkins.cilium.io/job/Cilium-PR-K8s-1.14-kernel-4.9/25/testReport/junit/Suite-k8s-1/14/K8sIstioTest_Istio_Bookinfo_Demo_Tests_bookinfo_inter_service_connectivity/

Restarting 1.14:
test-1.14-4.9

@pchaigno
Copy link
Member

test-1.16-4.9

@pchaigno
Copy link
Member

K8s-1.16-kernel-4.9 failed with known flakes #13773 and #13774 as well. K8s-1.14-kernel-4.9 had failed with #14958 and now failed with #14959.

Given the changes backported here (docs and labelsfilter), it's unlikely any of the two new flakes (#14958 and #14959) were introduced by this PR. I think we're ready to merge.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 12, 2021
@borkmann borkmann merged commit 92738e7 into v1.9 Feb 12, 2021
@borkmann borkmann deleted the pr/v1.9-backport-2021-02-11 branch February 12, 2021 14:30
@christarazi christarazi mentioned this pull request Mar 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@pchaigno pchaigno pchaigno approved these changes

Assignees
No one assigned
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@michi-covalent @joestringer @pchaigno @borkmann @ArthurChiao