Add encrypt-node status to cilium status #24399

romanspb80 · 2023-03-15T22:17:00Z

This patch adds param "NodeEncryption" to the cilium status

$ cilium status
[...]
Encryption: Wireguard [NodeEncryption: true, cilium_wg0 (Pubkey: UzRSJUmRqsxl82xvFG0joBheaiqOnM3BCgpByYjVtCE=, Port: 51871, Peers: 3)]

Fixes: #24239

cmd: Add NodeEncryption status to the cilium status command

nathanjsweet

Overall this looks good, but some minor fixes are needed. Thanks for the submission!

cilium-health/cmd/get.go

pkg/client/client.go

maintainer-s-little-helper · 2023-03-18T11:21:58Z

Commit e288da6f20df73e11d49b03089998529d38c84bf 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

romanspb80 · 2023-03-18T11:29:10Z

Thanks Nathan
Done

nathanjsweet

nit: typos

cilium-health/cmd/get.go

cilium/cmd/debuginfo.go

cilium/cmd/status.go

pkg/client/client.go

nathanjsweet · 2023-03-21T15:16:17Z

/test

Job 'Cilium-PR-K8s-1.25-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentPolicyTest 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.25-kernel-4.19 so I can create one.

gandro

Thanks for the PR. Unfortunately, this approach will not work. I recommend implementing the logic in the WireGuard status getter here:

cilium/pkg/wireguard/agent/agent.go

Line 567 in 02e61a1

func (a *Agent) Status(withPeers bool) (*models.WireguardStatus, error) {

cilium-health/cmd/get.go

gandro

Thanks! This is looking much better. There are two issues with the API definition that needs to be addressed, but the high-level approach looks good to me.

api/v1/models/wireguard_interface.go

pkg/client/client.go

gandro

Thanks! Looks good in terms of API, but local testing revealed that the formatting seem off now.

pkg/client/client.go

gandro

Awesome, thanks! One last nit, then this should be good to go.

pkg/client/client.go

This patch adds param "NodeEncryption" to the cilium status $ cilium status [...] Encryption: Wireguard [NodeEncryption: Enabled, cilium_wg0 (Pubkey: IoCvq0AB81XznieMpRyBl4XRcRxFuaN3Xhqefun5YQA=, Port: 51871, Peers: 0)] [...] Fixes: cilium#24239 Signed-off-by: Roman Ptitcyn <romanspb@yahoo.com>

gandro

Awesome, thanks a lot for tackling this!

gandro · 2023-04-03T08:25:55Z

/test

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

Click to show.

Test Name

K8sDatapathBandwidthTest Checks Bandwidth Rate-Limiting Checks Pod to Pod bandwidth, vxlan tunneling

Failure Output

FAIL: Found 2 io.cilium/app=operator logs matching list of errors that must be investigated:

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

gandro · 2023-04-03T11:08:16Z

https://jenkins.cilium.io/job/Cilium-PR-K8s-1.24-kernel-5.4/1556/ hit known flake CI: Suite-k8s-1.26.K8sUpdates Tests upgrade and downgrade from a Cilium stable image to master #24687
https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/1608/ hit unrelated variant of flake CI: K8sDatapathServicesTest Checks E/W loadbalancing (ClusterIP, NodePort from inside cluster, etc) Checks in-cluster KPR Tests HealthCheckNodePort #24701

gandro · 2023-04-03T12:05:37Z

Failures in required tests are unrelated (see above). Marking ready to merge.

romanspb80 · 2023-04-03T12:07:55Z

Failures in required tests are unrelated (see above). Marking ready to merge.

Thanks a lot

romanspb80 requested review from a team as code owners March 15, 2023 22:17

romanspb80 requested a review from nathanjsweet March 15, 2023 22:17

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 Mar 15, 2023

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Mar 15, 2023

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch 3 times, most recently from 075d500 to 2b33698 Compare March 17, 2023 18:29

nathanjsweet removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Mar 17, 2023

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 Mar 17, 2023

nathanjsweet added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Mar 17, 2023

nathanjsweet requested changes Mar 17, 2023

View reviewed changes

cilium-health/cmd/get.go Outdated Show resolved Hide resolved

pkg/client/client.go Outdated Show resolved Hide resolved

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch from 2b33698 to 1b4013c Compare March 18, 2023 11:18

maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 18, 2023

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch from e288da6 to 3ab4998 Compare March 18, 2023 11:24

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 18, 2023

romanspb80 requested a review from nathanjsweet March 18, 2023 11:59

nathanjsweet requested changes Mar 21, 2023

View reviewed changes

cilium-health/cmd/get.go Outdated Show resolved Hide resolved

cilium/cmd/debuginfo.go Outdated Show resolved Hide resolved

cilium/cmd/status.go Outdated Show resolved Hide resolved

pkg/client/client.go Outdated Show resolved Hide resolved

pkg/client/client.go Outdated Show resolved Hide resolved

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch from 3ab4998 to ef3d926 Compare March 21, 2023 15:12

nathanjsweet approved these changes Mar 21, 2023

View reviewed changes

nathanjsweet added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Mar 21, 2023

gandro requested changes Mar 21, 2023

View reviewed changes

cilium-health/cmd/get.go Outdated Show resolved Hide resolved

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch from ef3d926 to 9086262 Compare March 25, 2023 22:40

romanspb80 requested a review from a team as a code owner March 25, 2023 22:40

romanspb80 requested a review from brb March 25, 2023 22:40

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch 2 times, most recently from a53795e to 4ce05db Compare March 25, 2023 23:06

romanspb80 requested review from gandro and removed request for brb March 25, 2023 23:35

gandro reviewed Mar 27, 2023

View reviewed changes

api/v1/models/wireguard_interface.go Outdated Show resolved Hide resolved

gandro requested changes Mar 27, 2023

View reviewed changes

pkg/client/client.go Outdated Show resolved Hide resolved

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch from 4ce05db to 6a40980 Compare March 27, 2023 22:05

romanspb80 requested a review from gandro March 27, 2023 22:09

gandro requested changes Mar 28, 2023

View reviewed changes

pkg/client/client.go Outdated Show resolved Hide resolved

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch from 6a40980 to 57c7b62 Compare March 29, 2023 18:22

romanspb80 requested a review from gandro March 29, 2023 18:27

gandro reviewed Mar 30, 2023

View reviewed changes

pkg/client/client.go Outdated Show resolved Hide resolved

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch from 57c7b62 to d5d5e61 Compare March 31, 2023 17:54

romanspb80 force-pushed the Add_encrypt-node_status_to_cilium_status branch from d5d5e61 to e465b57 Compare March 31, 2023 17:54

romanspb80 requested a review from gandro March 31, 2023 17:55

gandro approved these changes Apr 3, 2023

View reviewed changes

gandro added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 3, 2023

squeed merged commit c601b50 into cilium:master Apr 4, 2023
41 of 42 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add encrypt-node status to cilium status #24399

Add encrypt-node status to cilium status #24399

romanspb80 commented Mar 15, 2023 •

edited by nathanjsweet

nathanjsweet left a comment

maintainer-s-little-helper bot commented Mar 18, 2023

romanspb80 commented Mar 18, 2023

nathanjsweet left a comment •

edited

nathanjsweet commented Mar 21, 2023 •

edited by maintainer-s-little-helper bot

Test Name

Failure Output

gandro left a comment

gandro left a comment •

edited

gandro left a comment

gandro left a comment

gandro left a comment

gandro commented Apr 3, 2023 •

edited by maintainer-s-little-helper bot

Test Name

Failure Output

gandro commented Apr 3, 2023 •

edited

gandro commented Apr 3, 2023

romanspb80 commented Apr 3, 2023

Add encrypt-node status to cilium status #24399

Add encrypt-node status to cilium status #24399

Conversation

romanspb80 commented Mar 15, 2023 • edited by nathanjsweet

nathanjsweet left a comment

Choose a reason for hiding this comment

maintainer-s-little-helper bot commented Mar 18, 2023

romanspb80 commented Mar 18, 2023

nathanjsweet left a comment • edited

Choose a reason for hiding this comment

nathanjsweet commented Mar 21, 2023 • edited by maintainer-s-little-helper bot

Test Name

Failure Output

gandro left a comment

Choose a reason for hiding this comment

gandro left a comment • edited

Choose a reason for hiding this comment

gandro left a comment

Choose a reason for hiding this comment

gandro left a comment

Choose a reason for hiding this comment

gandro left a comment

Choose a reason for hiding this comment

gandro commented Apr 3, 2023 • edited by maintainer-s-little-helper bot

Test Name

Failure Output

gandro commented Apr 3, 2023 • edited

gandro commented Apr 3, 2023

romanspb80 commented Apr 3, 2023

romanspb80 commented Mar 15, 2023 •

edited by nathanjsweet

nathanjsweet left a comment •

edited

nathanjsweet commented Mar 21, 2023 •

edited by maintainer-s-little-helper bot

gandro left a comment •

edited

gandro commented Apr 3, 2023 •

edited by maintainer-s-little-helper bot

gandro commented Apr 3, 2023 •

edited