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

API and implementation to quarantine service backends #18814

Merged

Conversation

aditighag
Copy link
Member

@aditighag aditighag commented Feb 15, 2022

The API will allow users to set backend states. The primary motivation is to quarantine unreachable backends so that such
backends won't be selected for load-balancing service traffic.
The backend states can be set in response to various events like Kubernetes events (e.g., terminating endpoints), or service APIs (e.g., quarantine unreachable backends). Backend states are persisted across cilium agent restart.

$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (active)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ cilium service update --backends 10.244.0.58:80 --states quarantined
Updating backend states
Updated service with 1 backends

$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (quarantined)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ cilium service update --backends 10.244.0.58:80 --states active
Updating backend states
Updated service with 1 backends

$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (active)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

See commits description for more details.

Support setting service backend states such as quarantine, maintenance so that these backends are not selected for load-balancing service traffic. 

@aditighag aditighag added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Feb 15, 2022
@aditighag aditighag requested review from a team February 15, 2022 17:46
@aditighag aditighag requested a review from a team as a code owner February 15, 2022 17:46
@aditighag aditighag marked this pull request as draft February 15, 2022 17:46
@aditighag
Copy link
Member Author

/test

@aditighag aditighag force-pushed the pr/aditighag/quarantine-lb-backends-api branch from 05ea342 to 89816f4 Compare February 15, 2022 23:13
@aditighag
Copy link
Member Author

/ci-l4lb

@aditighag aditighag force-pushed the pr/aditighag/quarantine-lb-backends-api branch from 89816f4 to ce8955d Compare February 23, 2022 06:36
@aditighag
Copy link
Member Author

/test

@aditighag aditighag marked this pull request as ready for review February 25, 2022 19:51
@aditighag aditighag requested a review from a team as a code owner February 25, 2022 19:51
@aditighag aditighag marked this pull request as draft February 28, 2022 22:57
@aditighag aditighag force-pushed the pr/aditighag/quarantine-lb-backends-api branch from ce8955d to 56c1d7d Compare March 1, 2022 00:53
@aditighag
Copy link
Member Author

/ci-l4lb

@aditighag
Copy link
Member Author

/test

@aditighag aditighag marked this pull request as ready for review March 4, 2022 02:19
@aditighag aditighag force-pushed the pr/aditighag/quarantine-lb-backends-api branch from 6d2fdc9 to 6399f12 Compare March 4, 2022 02:29
The API will allow users to update backend states. The primary
motivation is to quarantine unreachable backends so that such
backends won't be selected for load-balancing service traffic.
Internally, the agent iterate over the services map, and update all
the services that select the updated backends. States transitions
are internally validated before updating the states.

See dump below to how to set "quarantine" state for backends.
Quarantined backends when become reachable can be set back to
the active state.

```

$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (active)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ cilium service update --backends 10.244.0.58:80 --states quarantined
Updating backend states
Updated service with 1 backends

$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (quarantined)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ cilium service update --backends 10.244.0.58:80 --states active
Updating backend states
Updated service with 1 backends

$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (active)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ cilium service update --backends 10.244.0.58:80 --states terminating
Updating backend states
Updated service with 1 backends

e$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (terminating)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ cilium service update --backends 10.244.0.58:80 --states active
Updating backend states
Error: Cannot add/update service: [PUT /service/{id}][501] putServiceIdUpdateBackendFailure  invalid state transition for backend[10.244.0.58:80] (terminating) -> (active) &{BackendAddresses:[0xc0000f8990] Flags:0xc0001004b0 FrontendAddress:0xc00012f9c0 ID:0}

```

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Use the padding bits as flags. This is a
preparatory commit for the subsequent commits to be
able to encode backend state in the flags field which
can be restored after agent restart.

Signed-off-by: Aditi Ghag <aditi@cilium.io>
The `flags` field in the backend map value will be
used to encode the backend's state. This helps in
restoring backend states after agent restart.

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Backend states can change until the backends are deleted.
The new states need to be updated in the BPF backend map
so that the last updated state can be restored after agent restart.

```
$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (active)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ cilium service update --backends 10.244.0.58:80 --states quarantined
Updating backend states
Updated service with 1 backends

$ sudo bpftool map dump pinned /sys/fs/bpf/tc/globals/cilium_lb4_backends_v2
key: 03 00 00 00  value: 0a f4 01 08 00 50 00 00
key: 02 00 00 00  value: 0a f4 01 3b 00 50 00 00
key: 01 00 00 00  value: 0a f4 00 3a 00 50 00 02

$ cilium service update --backends 10.244.0.58:80 --states active
Updating backend states
Updated service with 1 backends

$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (active)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ sudo bpftool map dump pinned /sys/fs/bpf/tc/globals/cilium_lb4_backends_v2
key: 03 00 00 00  value: 0a f4 01 08 00 50 00 00
key: 02 00 00 00  value: 0a f4 01 3b 00 50 00 00
key: 01 00 00 00  value: 0a f4 00 3a 00 50 00 00
Found 3 elements

```

Signed-off-by: Aditi Ghag <aditi@cilium.io>
This commit builds up on previous commits where
we restore non-active backends after agent restart.
The non-active backend states such as `Quarantined` will
not be synced with Kubernetes events as these are set with
the Cilium service API. As a result, we need to persist backend
state in BPF maps, and restore it after agent restart.

When the agent is restarted, we first restore backends and services
from BPF maps. The persisted backends are marked as restored from
datapath. Later, once the agent's sync with Kubernetes API server
is complete, we reconcile the state restored from BPF maps with the service
and endpoint related events received from Kubernetes. The backend flag that
tracks restored backends from datapath is used to restore backend state
during this phase.

Testing -

Before agent restart -

```
$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (active)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

$ sudo bpftool map dump pinned /sys/fs/bpf/tc/globals/cilium_lb4_backends_v2
key: 03 00 00 00  value: 0a f4 01 08 00 50 00 00
key: 02 00 00 00  value: 0a f4 01 3b 00 50 00 00
key: 01 00 00 00  value: 0a f4 00 3a 00 50 00 00

$ cilium service update --backends 10.244.0.58:80 --states quarantined
Updating backend states
Updated service with 1 backends

$ sudo bpftool map dump pinned /sys/fs/bpf/tc/globals/cilium_lb4_backends_v2
key: 03 00 00 00  value: 0a f4 01 08 00 50 00 00
key: 02 00 00 00  value: 0a f4 01 3b 00 50 00 00
key: 01 00 00 00  value: 0a f4 00 3a 00 50 00 02
Found 3 elements

$ sudo systemctl restart cilium

```

After restart -

```

$ sudo bpftool map dump pinned /sys/fs/bpf/tc/globals/cilium_lb4_backends_v2
key: 03 00 00 00  value: 0a f4 01 08 00 50 00 00
key: 02 00 00 00  value: 0a f4 01 3b 00 50 00 00
key: 01 00 00 00  value: 0a f4 00 3a 00 50 00 02
Found 3 elements

$ cilium service list
ID   Frontend          Service Type   Backend
4    10.96.95.246:80   ClusterIP      1 => 10.244.0.58:80 (quarantined)
                                      2 => 10.244.1.59:80 (active)
                                      3 => 10.244.1.8:80 (active)

```

Signed-off-by: Aditi Ghag <aditi@cilium.io>
Validates service API to update backend states reflects
updated state in services list. Also, tests restore workflow
after agent restart.

Signed-off-by: Aditi Ghag <aditi@cilium.io>
@aditighag aditighag added release-note/major This PR introduces major new functionality to Cilium. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Apr 21, 2022
@aditighag
Copy link
Member Author

All tests except the vagrant ones have passed. Latest commits include a simple change - #18814 (comment).

@aditighag aditighag force-pushed the pr/aditighag/quarantine-lb-backends-api branch from be39ab6 to 49a6808 Compare April 21, 2022 06:25
@aditighag
Copy link
Member Author

aditighag commented Apr 21, 2022

/test

Job 'Cilium-PR-K8s-GKE' failed:

Click to show.

Test Name

K8sHealthTest cilium-health Checks status between nodes

Failure Output

FAIL: Expected

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

@aditighag
Copy link
Member Author

/test-1.23-net-next

@aditighag
Copy link
Member Author

/test-1.21-5.4

@aditighag
Copy link
Member Author

/test-vagrant

@aditighag
Copy link
Member Author

aditighag commented Apr 21, 2022

@aditighag
Copy link
Member Author

aditighag commented Apr 21, 2022

/test-gke

The previous test phrase didn't re-trigger the job for some reason.

@aditighag
Copy link
Member Author

aditighag commented Apr 21, 2022

/test-1.21-5.4

Previous -

14:42:56      k8s1-1.21: E: Could not get lock /var/lib/dpkg/lock-frontend. It is held by process 5238 (apt-get)
14:42:56      k8s1-1.21: E: Unable to acquire the dpkg frontend lock (/var/lib/dpkg/lock-frontend), is another process using it?

Job 'Cilium-PR-K8s-GKE' failed:

Click to show.

Test Name

K8sHealthTest cilium-health Checks status between nodes

Failure Output

FAIL: Expected

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

@aditighag
Copy link
Member Author

aditighag commented Apr 21, 2022

/test-gke

Job 'Cilium-PR-K8s-1.23-kernel-net-next' failed:

Click to show.

Test Name

K8sUpdates Tests upgrade and downgrade from a Cilium stable image to master

Failure Output

FAIL: terminating containers are not deleted after timeout

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-net-next so I can create one.

@aditighag
Copy link
Member Author

/test-runtime

@aditighag
Copy link
Member Author

net-next hit #18873.

@aditighag aditighag added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 22, 2022
@tklauser tklauser merged commit 4b52130 into cilium:master Apr 22, 2022
jrajahalme added a commit to jrajahalme/cilium that referenced this pull request Apr 22, 2022
Use the backend 'flags' field instead the old 'pad'. This fixes bpf compilation fails when L7 LB is used, like this:

```
level=warning msg="/var/lib/cilium/bpf/bpf_sock.c:387:14: error: no member named 'pad' in 'struct lb4_backend'" subsys=datapath-loader
level=warning msg="                        l7backend.pad = 0;" subsys=datapath-loader
level=warning msg="                        ~~~~~~~~~ ^" subsys=datapath-loader
level=warning msg="1 error generated." subsys=datapath-loader
```

Add ENABLE_L7_LB to LB_OPTIONS in bpf/Makefile to catch this kind of errors in future.

Fixes: cilium#18814
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
joestringer pushed a commit that referenced this pull request Apr 22, 2022
Use the backend 'flags' field instead the old 'pad'. This fixes bpf compilation fails when L7 LB is used, like this:

```
level=warning msg="/var/lib/cilium/bpf/bpf_sock.c:387:14: error: no member named 'pad' in 'struct lb4_backend'" subsys=datapath-loader
level=warning msg="                        l7backend.pad = 0;" subsys=datapath-loader
level=warning msg="                        ~~~~~~~~~ ^" subsys=datapath-loader
level=warning msg="1 error generated." subsys=datapath-loader
```

Add ENABLE_L7_LB to LB_OPTIONS in bpf/Makefile to catch this kind of errors in future.

Fixes: #18814
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.12 This issue will prevent the release of the next version of Cilium. release-note/major This PR introduces major new functionality to Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants