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

OSDOCS-10856: Add bpfman Operator #80208

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Conversation

@jab-rh jab-rh self-assigned this Aug 8, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Aug 8, 2024

@jab-rh: This pull request references OSDOCS-10856 which is a valid jira issue.

In response to this:

Version(s):

Issue:

Link to docs preview:

QE review:

  • QE has approved this change.

Additional information:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Aug 8, 2024
@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 8, 2024
@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 8, 2024
@jab-rh
Copy link
Contributor Author

jab-rh commented Sep 4, 2024

@skrthomas, this PR covers the bpfman Operator installation and usage. Somehow this aligns with OSDOCS-10882.

@jab-rh
Copy link
Contributor Author

jab-rh commented Sep 4, 2024

@msherif1234, this is a documentation draft for the bfpman Operator that discusses installation and a high level overview of usage for the upcoming 4.17 release. Can you take a look when you have time? Thanks!

@jab-rh
Copy link
Contributor Author

jab-rh commented Sep 4, 2024

@anuragthehatter, can you take a look when you have some time? Thanks!

@jab-rh
Copy link
Contributor Author

jab-rh commented Sep 4, 2024

@bmcelvee, this fails on ROSA builds; How do I know if I should be including this in the ROSA topic map? Thanks!

@openshift-ci-robot
Copy link

openshift-ci-robot commented Sep 4, 2024

@jab-rh: This pull request references OSDOCS-10856 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.18." or "openshift-4.18.", but it targets "openshift-4.17" instead.

In response to this:

Version(s):

Issue:

Link to docs preview:

QE review:

  • QE has approved this change.

Additional information:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@skrthomas
Copy link
Contributor

@jab-rh thanks. Its my understanding that the NetObserv piece for this is only coming in 4.18 timeframe. That's as of maybe a couple weeks ago when I asked.

@bmcelvee
Copy link
Contributor

bmcelvee commented Sep 4, 2024

Since this is tech preview, I don't believe it will be applicable to ROSA until it fully GAs. Conditionalizing the content should clear the build errors (ifndef for ROSA and OSD). @rafael-azevedo would you be able to confirm or point us to someone who can? Thanks!

@jab-rh jab-rh changed the title OSDOCS-10856: Add bpfman Operator for ingress firewalls OSDOCS-10856: Add bpfman Operator Sep 12, 2024
[id="bpfman-infw-configure_{context}"]
= Configuring Ingress Node Firewall Operator to use the bpfman Operator

As a cluster administrator, you can configure the Ingress Node Firewall Operator to the use bpfman Operator to manage BPF programs.

Choose a reason for hiding this comment

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

I think this is a confusing way to say this. It's more that we are using bpfman to manage INFW's bpf

Copy link
Member

Choose a reason for hiding this comment

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

Here's a thought on some potential alternative language?

Suggested change
As a cluster administrator, you can configure the Ingress Node Firewall Operator to the use bpfman Operator to manage BPF programs.
The Ingress Node Firewall employs [BPF](https://www.kernel.org/doc/html/latest/bpf/index.html) programs to implement some of its key firewall functionality. By default these BPF programs are loaded into the Kernel using a mechanism specific to the Ingress Node Firewall but the operator can be optionally configured to use [bpfman](https://bpfman.io) to load and manage these programs instead, adding additional security and observability functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shaneutt thanks!

modules/nw-bpfman-operator-deploy.adoc Show resolved Hide resolved
@@ -0,0 +1,93 @@
// Module included in the following assemblies:

Choose a reason for hiding this comment

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

I think we have this issue with other operators -- but why are we including the documentation here to install bpfman rather than link to the bpfman documentation itself?

[id="nw-bpfman-operator-installing-console_{context}"]
= Installing the bpfman Operator using the web console

As a cluster administrator, you can install the bpfman Operator using the web console.

Choose a reason for hiding this comment

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

I think we have this issue with other operators -- but why are we including the documentation here to install bpfman rather than link to the bpfman documentation itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davegord, does the upstream documentation describe how to install this on Red Hat OpenShift?

Choose a reason for hiding this comment

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

I doubt it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davegord, historically we've always documented how to install Operators on Red Hat OpenShift in our OpenShift documentation. I can ask my content strategist if we can omit this, but I think including it provides a better user experience.

eBPF CRDs: A set of CRDs like XdpProgram and TcProgram for loading eBPF programs, and a bpfman-generated CRD (BpfProgram) for representing the state of loaded programs.
bpfman-agent:: Runs within a daemonset container, ensuring eBPF programs on each node are in the desired state.
bpfman-operator:: Manages the lifecycle of the bpfman-agent and CRDs in the cluster using the Operator SDK.

Choose a reason for hiding this comment

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

This whole section I think needs to be either reworked or removed. We shouldn't call this an "open source project". I assume this was taken from the bpfman.io site but this is not appropriate for our downstream/product documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davegord, sure, I'm open to suggestions for what content to include here. Thanks!

Choose a reason for hiding this comment

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

I think it's based on some upstream content, but not word for word. Is anyone going to take a pass at it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anfredette, a pass, in so far as, this content is incorrect or not relevant here? Thanks!

@jab-rh
Copy link
Contributor Author

jab-rh commented Sep 16, 2024

@anuragthehatter, ping

Copy link

@anfredette anfredette left a comment

Choose a reason for hiding this comment

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

I just have a few comments.

--
where:

`<ebpf_mode>`: Specifies whether the Ingress Node Firewall Operator uses the bpfman Operator to manage BPF programs.

Choose a reason for hiding this comment

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

Do you need to mention this can be true or false?

modules/nw-bpfman-operator-installing-cli.adoc Outdated Show resolved Hide resolved
eBPF CRDs: A set of CRDs like XdpProgram and TcProgram for loading eBPF programs, and a bpfman-generated CRD (BpfProgram) for representing the state of loaded programs.
bpfman-agent:: Runs within a daemonset container, ensuring eBPF programs on each node are in the desired state.
bpfman-operator:: Manages the lifecycle of the bpfman-agent and CRDs in the cluster using the Operator SDK.

Choose a reason for hiding this comment

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

I think it's based on some upstream content, but not word for word. Is anyone going to take a pass at it?

@jab-rh jab-rh added the peer-review-needed Signifies that the peer review team needs to review this PR label Sep 17, 2024
+
[source,terminal]
----
$ curl https://github.com/bpfman/bpfman/releases/download/v0.5.0/go-xdp-counter-install-selinux.yaml -o go-xdp-counter-install-selinux.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

There are a few spots in these docs where the links are to v0.5.0. They should globally be changed to v0.5.1. Hopefully before docs freeze we will have a v0.5.2 and they can change to v0.5.2, but they should be moved to v0.5.1 for now.

Suggested change
$ curl https://github.com/bpfman/bpfman/releases/download/v0.5.0/go-xdp-counter-install-selinux.yaml -o go-xdp-counter-install-selinux.yaml
$ curl https://github.com/bpfman/bpfman/releases/download/v0.5.1/go-xdp-counter-install-selinux.yaml -o go-xdp-counter-install-selinux.yaml

modules/nw-bpfman-operator-deploy.adoc Outdated Show resolved Hide resolved
modules/nw-bpfman-operator-deploy.adoc Outdated Show resolved Hide resolved
modules/nw-bpfman-operator-deploy.adoc Show resolved Hide resolved
+
[source,terminal]
----
$ curl https://github.com/bpfman/bpfman/releases/download/v0.5.0/go-xdp-counter-install-selinux.yaml -o go-xdp-counter-install-selinux.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Test the curl command. I tried it and it created an empty file. I replaced curl with wget (removed the -o as well) and downloaded fine, so the link is valid. Not sure why the curl failed for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

I found the issue. The link is a redirect under the covers. wget follows redirects by default, curl does not. To fix the command, add a -L:

Suggested change
$ curl https://github.com/bpfman/bpfman/releases/download/v0.5.0/go-xdp-counter-install-selinux.yaml -o go-xdp-counter-install-selinux.yaml
$ curl -L https://github.com/bpfman/bpfman/releases/download/v0.5.0/go-xdp-counter-install-selinux.yaml -o go-xdp-counter-install-selinux.yaml

modules/nw-bpfman-infw-configure.adoc Show resolved Hide resolved

.Procedure

* Edit the `IngressNodeFirewallConfig` object named `ingressnodefirewallconfig` and set `ebpfProgramManagerMode` field:
Copy link
Contributor

Choose a reason for hiding this comment

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

@davegord I think within OCP, bpfman is named "eBPF Program Manager", or something like that? Should all references to bpfman in OCP docs use that?

@jldohmann jldohmann added the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Sep 17, 2024
@jldohmann
Copy link
Contributor

@jab-rh what versions of the docs does this need to be published in?

Copy link
Contributor

@jldohmann jldohmann left a comment

Choose a reason for hiding this comment

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

nice work! 😃 mostly nits and minor comments below

modules/nw-bpfman-infw-configure.adoc Outdated Show resolved Hide resolved
modules/nw-bpfman-infw-configure.adoc Outdated Show resolved Hide resolved
modules/nw-bpfman-infw-configure.adoc Outdated Show resolved Hide resolved
modules/nw-bpfman-infw-configure.adoc Show resolved Hide resolved
selinuxprofile.security-profiles-operator.x-k8s.io/bpfman-secure created
----

. To confirm that the eBPF sample application deployed successfully, enter the following command:
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be nice to group all these confirmation steps into a single step (with substeps) or a separated verification section

Copy link
Contributor

@Billy99 Billy99 left a comment

Choose a reason for hiding this comment

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

Saw a few more bpfman instances.

Copy link

openshift-ci bot commented Sep 30, 2024

@jab-rh: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@kalexand-rh
Copy link
Contributor

The branch/enterprise-4.18 label has been added to this PR.

This is because your PR targets the main branch and is labeled for enterprise-4.17. And any PR going into main must also target the latest version branch (enterprise-4.18).

If the update in your PR does NOT apply to version 4.18 onward, please re-target this PR to go directly into the appropriate version branch or branches (enterprise-4.x) instead of main.

Copy link

@anfredette anfredette left a comment

Choose a reason for hiding this comment

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

/LGTM

Copy link

openshift-ci bot commented Sep 30, 2024

@anfredette: changing LGTM is restricted to collaborators

In response to this:

/LGTM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@msherif1234
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2024
Copy link
Contributor

@Billy99 Billy99 left a comment

Choose a reason for hiding this comment

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

/lgtm
We can follow-up later with a list of limitations of running INFW with eBPF Manager (maybe we can have those fixed by then).

@kalexand-rh
Copy link
Contributor

@Billy99, please open a new issue in the OSDOCS project to capture what you'd like to see with that list of limitations.

@anuragthehatter
Copy link

/lgtm

@kalexand-rh kalexand-rh merged commit 6138b69 into openshift:main Sep 30, 2024
2 checks passed
@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.17

@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.18

@openshift-cherrypick-robot

@kalexand-rh: new pull request created: #82744

In response to this:

/cherrypick enterprise-4.17

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-cherrypick-robot

@kalexand-rh: new pull request created: #82745

In response to this:

/cherrypick enterprise-4.18

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@Billy99
Copy link
Contributor

Billy99 commented Sep 30, 2024

@Billy99, please open a new issue in the OSDOCS project to capture what you'd like to see with that list of limitations.

#82746

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.17 branch/enterprise-4.18 jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.