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

Display host firewall status in cilium status #17165

Merged
merged 1 commit into from Sep 8, 2021

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented Aug 16, 2021

With this pull request, the cilium status command will display e.g.:

KVStore:                Ok   Disabled
Kubernetes:             Ok   1.21 (v1.21.0) [linux/amd64]
Kubernetes APIs:        ["cilium/v2::CiliumClusterwideNetworkPolicy", "cilium/v2::CiliumEndpoint", "cilium/v2::CiliumNetworkPolicy", "cilium/v2::CiliumNode", "core/v1::Namespace", "core/v1::Node", "core/v1::Pods", "core/v1::Service", "discovery/v1::EndpointSlice", "networking.k8s.io/v1::NetworkPolicy"]
KubeProxyReplacement:   Disabled
Host firewall:          Enabled    [enp0s3, enp0s8]
Cilium:                 Ok   1.10.90 (v1.10.90-3e9722fd5c)
NodeMonitor:            Listening for events on 2 CPUs with 64x4096 of shared memory
Cilium health daemon:   Ok
IPAM:                   IPv4: 3/254 allocated from 10.11.0.0/24, IPv6: 3/65534 allocated from fd00::/112
BandwidthManager:       EDT with BPF   [enp0s3, enp0s8]
Host Routing:           Legacy
Masquerading:           IPTables [IPv4: Enabled, IPv6: Enabled]
Controller Status:      23/23 healthy
Proxy Status:           OK, ip 10.11.0.62, 0 redirects active on ports 10000-20000
Hubble:                 Ok   Current/Max Flows: 4095/4095 (100.00%), Flows/s: 21.86   Metrics: Disabled
Encryption:             Disabled
Cluster health:         1/1 reachable   (2021-08-16T09:11:14Z)

Fixes: #14749.

@pchaigno pchaigno added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. area/cli Impacts the command line interface of any command in the repository. area/host-firewall Impacts the host firewall or the host endpoint. labels Aug 16, 2021
@pchaigno pchaigno changed the title Display host firewall status in cilium status Display host firewall status in cilium status Aug 16, 2021
@pchaigno pchaigno marked this pull request as ready for review August 16, 2021 14:28
@pchaigno pchaigno requested a review from a team August 16, 2021 14:28
@pchaigno pchaigno requested a review from a team as a code owner August 16, 2021 14:28
@pchaigno pchaigno requested a review from a team August 16, 2021 14:28
@pchaigno pchaigno requested a review from a team as a code owner August 16, 2021 14:28
@pchaigno pchaigno requested a review from brb August 20, 2021 16:31
Copy link
Member

@borkmann borkmann left a comment

Choose a reason for hiding this comment

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

Host firewall: Enabled [enp0s3 10.0.2.15 fd00::b, enp0s8 192.168.33.11 fd01::b]

What is the context of the IP addresses related to HF? Shouldn't it be independent which L3 address is configured there, meaning, only configured devices (enp0s3, enp0s8) should be of interest? (Or are these NodePort related addresses?)

@pchaigno
Copy link
Member Author

Host firewall: Enabled [enp0s3 10.0.2.15 fd00::b, enp0s8 192.168.33.11 fd01::b]

What is the context of the IP addresses related to HF? Shouldn't it be independent which L3 address is configured there, meaning, only configured devices (enp0s3, enp0s8) should be of interest? (Or are these NodePort related addresses?)

More or less. We do still rely on the IP addresses to know if the traffic entering the node is directed to the host namespace (based on HOST_ID).

@brb
Copy link
Member

brb commented Sep 7, 2021

More or less. We do still rely on the IP addresses to know if the traffic entering the node is directed to the host namespace (based on HOST_ID).

@pchaigno Don't we use IPCache for that? If yes, then I assume that if any of those devices have multiple IP addrs (e.g., k8s ExternalIP and InternalIP), then all of them should be in the IPCache regardless of those shown in the status.

Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

LGTM, just one question regarding the IP addrs in the HF status.

@pchaigno
Copy link
Member Author

pchaigno commented Sep 7, 2021

More or less. We do still rely on the IP addresses to know if the traffic entering the node is directed to the host namespace (based on HOST_ID).

@pchaigno Don't we use IPCache for that? If yes, then I assume that if any of those devices have multiple IP addrs (e.g., k8s ExternalIP and InternalIP), then all of them should be in the IPCache regardless of those shown in the status.

Hm, you're right. The list of IP addresses is going to be misleading in that context. I'll remove it.

With this commit the "cilium status" command will display e.g.:

    KVStore:                Ok   Disabled
    Kubernetes:             Ok   1.21 (v1.21.0) [linux/amd64]
    Kubernetes APIs:        ["cilium/v2::CiliumClusterwideNetworkPolicy", "cilium/v2::CiliumEndpoint", "cilium/v2::CiliumNetworkPolicy", "cilium/v2::CiliumNode", "core/v1::Namespace", "core/v1::Node", "core/v1::Pods", "core/v1::Service", "discovery/v1::EndpointSlice", "networking.k8s.io/v1::NetworkPolicy"]
    KubeProxyReplacement:   Disabled
    Host firewall:          Enabled    [enp0s3, enp0s8]
    Cilium:                 Ok   1.10.90 (v1.10.90-3e9722fd5c)
    NodeMonitor:            Listening for events on 2 CPUs with 64x4096 of shared memory
    Cilium health daemon:   Ok
    IPAM:                   IPv4: 3/254 allocated from 10.11.0.0/24, IPv6: 3/65534 allocated from fd00::/112
    BandwidthManager:       EDT with BPF   [enp0s3, enp0s8]
    Host Routing:           Legacy
    Masquerading:           IPTables [IPv4: Enabled, IPv6: Enabled]
    Controller Status:      23/23 healthy
    Proxy Status:           OK, ip 10.11.0.62, 0 redirects active on ports 10000-20000
    Hubble:                 Ok   Current/Max Flows: 4095/4095 (100.00%), Flows/s: 21.86   Metrics: Disabled
    Encryption:             Disabled
    Cluster health:         1/1 reachable   (2021-08-16T09:11:14Z)

Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno
Copy link
Member Author

pchaigno commented Sep 7, 2021

test-me-please

@brb
Copy link
Member

brb commented Sep 7, 2021

@pchaigno GH shows me only one commit in this PR. Is it intended?

@pchaigno
Copy link
Member Author

pchaigno commented Sep 7, 2021

@pchaigno GH shows me only one commit in this PR. Is it intended?

Yes, the other two preparatory commits were only needed when displaying the IP addresses. I've updated the pull request description accordingly.

Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

🚀 (it would be nice if you could open a PR for that KPR little refactoring)

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Sep 7, 2021
@borkmann borkmann merged commit 37f352e into cilium:master Sep 8, 2021
@pchaigno pchaigno deleted the host-firewall-status branch September 8, 2021 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Impacts the command line interface of any command in the repository. area/host-firewall Impacts the host firewall or the host endpoint. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. 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.

Display host firewall status in cilium status
4 participants