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

Add Support for Namespace Override #23527

Closed
wants to merge 373 commits into from
Closed

Add Support for Namespace Override #23527

wants to merge 373 commits into from

Conversation

pr0PM
Copy link

@pr0PM pr0PM commented Feb 2, 2023

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

This is helpful while deploying charts using gitOps tools like argoCD. Usually a standard among all the charts.

Fixes: #20511

Signed-off-by: Prateek Mishra pr0PM@pm.me

Support for namespace override in cilium charts

@pr0PM pr0PM requested review from a team as code owners February 2, 2023 09:49
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 2, 2023
@pr0PM pr0PM requested a review from chancez February 2, 2023 09:49
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Feb 2, 2023
@pr0PM
Copy link
Author

pr0PM commented Feb 2, 2023

I think the docs would need an update too for this.

@sayboras sayboras added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/helm Impacts helm charts and user deployment experience labels Feb 2, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Feb 2, 2023
@sayboras
Copy link
Member

sayboras commented Feb 3, 2023

Seems like there is some template issue that you are checking, I am marking this as draft PR.

Feel free to ping in #development channel if you want to discuss or need any help. Thanks.

@sayboras sayboras marked this pull request as draft February 3, 2023 11:24
nbusseneau and others added 15 commits February 16, 2023 17:42
We removed support for kernel 4.9 in the CI via
bbda112, as part of ongoing work to
bump the minimal supprted kernel version (see [1]).

We have rotated / expanded the Jenkins test jobs as follow:

- Changed: K8s 1.16 on Kernel 4.19 (instead of 4.9, triggered on `/test`).
- Changed: K8s 1.17-1.24 on Kernel 4.19 (instead of 4.9, triggered on `/test-missed-k8s`).

See the Table of Truth™️ for up to date status on all trigger phrases:
https://docs.google.com/spreadsheets/d/1TThkqvVZxaqLR-Ela4ZrcJ0lrTJByCqrbdCjnI32_X0/edit#gid=0

[1]: #22116

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
Policy MapState needs to respect property authType when checking
it for dataPath equality. Without this change, changing the
auth type on existing CiliumNetworkPolicies isn't reflected
in the eBPF policy map.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
…" test.

Related: #22019

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
GKE testing via Jenkins has been disabled on `master` for several months
and is not tested anymore.

This commits removes the GKE pipeline from Jenkinsfiles and also removes
some GKE obsolete bits in MLH's config.

Signed-off-by: Nicolas Busseneau <nicolas@isovalent.com>
This flag is for Cilium to support K8s NetworkPolicy.
By default the value is true. User can set "enable-k8s-networkpolicy = false" to disable it.

Signed-off-by: Li Chengyuan <chengyuanli@hotmail.com>
Removes the dependency for bpf host routing to be enabled for SRv6 decap
to function correctly.

Prior to this change, srv6 packets arriving at a native dev, on a datapath
which did not enable bpf host routing, would slip into a feature check
and get sent to stack before SRv6 decap occurred.

Reorder SRv6 decap to occur before the bpf host routing check.

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
restoreServicesLocked() uses DumpServiceMaps() to get service maps entries, which can return services with some empty (nil) backends. Later it loops through service backends and accesses fields of pointers that can be nil. Previously, the Backends slice was holding objects, not pointers. Since #20410 change, it holds pointers, and this issue can occur.

Signed-off-by: Dorde Lapcevic <dordel@google.com>
This commit added a Cell for the certificate manager. The cell exposes
two new interfaces `CertificateManager` and `SecretManager` instead
of the manager directly.

The configuration/flags for the manager has been moved into the Cell.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
For TCP traffic, dsr_set_opt4() only inserts the DSR option into SYN
packets. When extracting the TCP flags, consider that the packet might
already contain unrelated IPv4 options and skip over them.

When the backend node later uses handle_dsr_v4() to check for the DSR IPv4
option, tolerate such unrelated options by relaxing the length check.
Note that we always insert the DSR option at the front, so it's sufficient
to parse the first option.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
IPv4 packets can only carry a certain amount of IPv4 options. Check if
we stay within the limit before inserting the DSR option.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Remove some dead code.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
We currently have IPv4 and IPv6 variants of several helpers that update a
CT entry. These only differ in the CT tuple parameter, which then gets used
as key for a map lookup.

To avoid this duplication, take ct_update_nodeport() as example and turn
the CT tuple into an opaque parameter.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Change the print modifier of the debug messages about etcd emitted
events to '%s', to print the retrieved value as human-readable string
rather than array of bytes.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Bill Mulligan <billmulligan516@gmail.com>
tklauser and others added 14 commits March 2, 2023 12:27
With the recent CoverBee version bump to v0.3.1 we can enable coverage
reporting for BPF tests again.

This reverts commit edd2abf (".github/workflows: disable coverage
in BPF tests").

Fixes #22088

Signed-off-by: Tobias Klauser <tobias@cilium.io>
I missed excluding these roles when the agent is disabled.

Fixes: 0dbde96
Fixes: #23819
Signed-off-by: Casey Callendrello <cdc@isovalent.com>
Signed-off-by: Tam Mach <tam.mach@cilium.io>
As L7 LB service is having dummy endpoint (e.g. 192.192.192.192), the
in-cluster traffic will be dropped due to no healthy backend. This commit
is to make sure that the proper exclusion rule is done.

Relates: #21871
Fixes: 38959d4

Signed-off-by: Tam Mach <tam.mach@cilium.io>
The install-iptables-rules flag is used by developpers to work on the
iptables-free "feature", but it shouldn't be changed by Cilium users.
Cilium is currently not completely functional without iptables rules.

Let's therefore hide it.

Signed-off-by: Paul Chaignon <paul@cilium.io>
We can't hide the Helm flag as we did for install-iptables-rules in the
previous commit, so let's just remove it.

Signed-off-by: Paul Chaignon <paul@cilium.io>
To wait for a CiliumEgressGatewayPolicy to be effective in a ginkgo test
suite, we rely on the WaitForEgressPolicyEntry helper, which lists the
entries in the bpf egress policy map and makes sure that at least one
entry matches the k8s3 node IP (as that's used as destination CIDR in
one of the policies).

This is not optimal as matching that IP doesn't guarantee that the full
policy/all policies are effective.

With this commit, instead of looking for a particular IP, we just count
the number of entries, since, given a policies manifest, it's easy to
determine how many entries we should expect in the bpf map.

While at it make sure also that we properly handle any error returned by
WaitForEgressPolicyEntries and that we wait for
kubectl.Delete(policyYAML) to actually remove any existing entry from
the bpf map.

Signed-off-by: Gilberto Bertin <jibi@cilium.io>
To retrieve the Cilium pod used to run the "cilium bpf egress list"
command, WaitForEgressPolicyEntries invokes GetCiliumPodOnNode(node),
which expects a node name, and not a node IP.

Signed-off-by: Gilberto Bertin <jibi@cilium.io>
Despite its title, commit 92eaa5b ("bpf: Add VTEP redirection to
complexity and compile tests") didn't cover the VTEP feature in compile
tests. This commit fixes it.

Fixes: 92eaa5b ("bpf: Add VTEP redirection to complexity and compile tests")
Signed-off-by: Paul Chaignon <paul@cilium.io>
We first look up the destination endpoint to check for tunnel redirection,
and then look it up a second time to access its sec_label and IPSec key.

Make the first look-up unconditional, so that we can remove the second.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
There is a `info == NULL` check just a few lines earlier. The verifier
should be smart enough to understand that we don't need this again.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
The package "github.com/cilium/cilium/pkg/option" was imported more than
once with different aliases. The commit fixes it removing the
unnecessary additional import.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
The support for L3 devices requires Linux 5.8 with commit 6f3f65d80d
("net: bpf: Allow TC programs to call BPF_FUNC_skb_change_head"). Let's
document that requirement.

Fixes: 27ae873 ("daemon: Ignore L3 devices on unsupported kernels")
Signed-off-by: Paul Chaignon <paul@cilium.io>
Signed-off-by: Renovate Bot <bot@renovateapp.com>
@pr0PM
Copy link
Author

pr0PM commented Mar 2, 2023

  • the values.yaml.tpl has been updated
  • docs updated
  • rebased the commit

Should be good to move forward.

@pr0PM
Copy link
Author

pr0PM commented Mar 6, 2023

Updated the broken lint on the cilium agent role-binding where terminal else condition was missing.

@pr0PM
Copy link
Author

pr0PM commented Mar 6, 2023

Need to resolve these test failures

@pr0PM pr0PM requested a review from a team as a code owner March 6, 2023 15:33
@pr0PM pr0PM requested a review from qmonnet March 6, 2023 15:33
@sayboras
Copy link
Member

sayboras commented Mar 7, 2023

There is a small mismatch on commit author and sign-off, commit author is with pr0PM pr0pm@pm.me, but sign-off is Prateek Mishra pr0PM@pm.me.

If you are pushing one more time, can you help to adjust both to Prateek Mishra pr0PM@pm.me ? Thanks a lot.

https://github.com/cilium/cilium/actions/runs/4345145130/jobs/7589732180

  Error: ERROR:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'pr0PM <pr0pm@pm.me>'

@maintainer-s-little-helper
Copy link

Commit cff239e953fe82952dfb72f2f7d72a351a66b73c does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. and removed dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. labels Mar 7, 2023
@sayboras sayboras requested a review from qmonnet March 7, 2023 05:59
pr0PM and others added 2 commits March 7, 2023 13:37
This is helpful while deploying charts using gitOps tools like
argoCD. Usually a standard among all the charts.

- Patch for the helper.tpl priority class issue
- document generation steps
- spellcheck allowlist
- make install/kubernetes
- Add doc comments

Fixes: #20511

Signed-off-by: Prateek Mishra <pr0PM@pm.me>
Signed-off-by: Prateek Mishra <pr0PM@pm.me>
@pr0PM pr0PM closed this Mar 7, 2023
@pr0PM
Copy link
Author

pr0PM commented Mar 7, 2023

Changed commit author this by editing the commit and replacing it. This blog helped.
Last time I just updated my global git config hence failed with same issue in bpf test.

Edit: Not sure why the PR got closed by editing the author.

Edit 2: I messed up, will fix it up soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Impacts helm charts and user deployment experience kind/community-contribution This was a contribution made by a community member. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CFP: Add a value for overriding the namespace in the helm chart.