OCPBUGS-78524: Add OCP-88729 verify USBGuard extension install and enable via MachineConfig#6034
Conversation
…neConfig Adds e2e test that creates two MachineConfigs on worker nodes: one to install the usbguard extension and another to enable the usbguard.service systemd unit, then verifies the service is enabled on all worker nodes. Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@HarshwardhanPatil07: No Jira issue with key OCP-88729 exists in the tracker at https://redhat.atlassian.net. DetailsIn response to this:
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughA new disruptive test case ( ChangesUSBGuard Installation and Enablement Test
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: HarshwardhanPatil07 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Logsharshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/machine-config-operator$ cat /tmp/claude-4242557/-home-harshpat-Downloads-repos-machine-config-operator/17de08d4-3a6b-4e2b-b222-8b71fc20f2b5/tasks/b0gbqgzgt.output I0513 11:42:42.563365 92129 test_context.go:566] The --provider flag is not set. Continuing as if --provider=skeleton had been used. Running Suite: - /home/harshpat/Downloads/repos/machine-config-operator ======================================================================== Random Seed: 1778652762 - will randomize all specs /verified by @HarshwardhanPatil07 |
openshift-ci-robot
added
the
verified
|
@HarshwardhanPatil07: This PR has been marked as verified by DetailsIn response to this:
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. |
|
@HarshwardhanPatil07: No Jira issue with key OCP-88729 exists in the tracker at https://redhat.atlassian.net. DetailsIn response to this:
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. |
|
/retitle OCPBUGS-78524: Add OCP-88729 verify USBGuard extension install and enable via MachineConfig |
openshift-ci
Bot
changed the title
openshift-ci-robot
added
jira/severity-important
|
@HarshwardhanPatil07: This pull request references Jira Issue OCPBUGS-78524, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
|
/jira-refresh |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended-priv/mco_security.go`:
- Line 991: The test "g.It" case that starts with "[PolarionID:88729][OTP]
Verify USBGuard..." calls MachineConfig APIs not present in MicroShift; update
the test name string in the g.It invocation (the test declared in
mco_security.go with the g.It containing "[PolarionID:88729][OTP] Verify
USBGuard extension...") to include the skip label "[Skipped:MicroShift]" so it
will not run on MicroShift clusters (i.e., insert the label into the first
string argument of the g.It call).
- Around line 999-1010: After creating the MachineConfigs, wait for the
MachineConfigPool rollout to finish before proceeding: after calling
mcExt.create() and after mcEnable.create() invoke mcp.waitForComplete() (or at
minimum call mcp.waitForComplete() once after both creations) so the config
rendering and node updates complete before the subsequent verification of
usbguard.service; reference the functions mcExt.create(), mcEnable.create(), and
mcp.waitForComplete() to locate where to add the waits.
- Around line 1011-1015: The loop that asserts node.IsUnitEnabled("usbguard")
should be wrapped in an o.Eventually to retry transient failures: replace the
direct loop over nodes (from mcp.GetSortedNodesOrFail()) with an o.Eventually
that runs a closure which re-fetches nodes (call mcp.GetSortedNodesOrFail()
inside the closure) and performs the for _, node := range nodes {
o.Expect(node.IsUnitEnabled("usbguard")).To(o.BeTrue(), "usbguard.service should
be enabled on node %s", node.GetName()) } checks; configure a reasonable timeout
and polling interval to match other tests in this file so node-level state has
time to converge.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 56c78d07-1cb4-4fef-a50f-0771354f12e4
📒 Files selected for processing (1)
test/extended-priv/mco_security.go
…private Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
openshift-ci-robot
removed
the
verified
…erification Address review feedback: move test from mco_security.go to mco_kernel.go since it tests extensions, and add verification that the usbguard RPM is installed on all worker nodes before checking the service is enabled. Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended-priv/mco_kernel.go`:
- Around line 339-372: The test "Verify USBGuard extension can be installed and
enabled via MachineConfig on worker nodes" uses MachineConfig (NewMachineConfig,
SetMCOTemplate, SetParams) which is not present on MicroShift; update the g.It
declaration to include the "[Skipped:MicroShift]" label in the test name OR add
a runtime guard using exutil.IsMicroShiftCluster() (early return/Skip) at the
start of the test body before calling
GetCompactCompatiblePool()/NewMachineConfig() so the test is skipped on
MicroShift clusters.
- Line 368: The assertion calls node.IsUnitEnabled with the abbreviated unit
name; update the call to use the full systemd unit name "usbguard.service" so it
matches the assertion message and other usages (e.g., change
node.IsUnitEnabled("usbguard") to node.IsUnitEnabled("usbguard.service") in the
test where the Expect(...).To(o.BeTrue()) check occurs).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d60b4adb-43fe-4fe5-bf73-66763daed3de
📒 Files selected for processing (2)
test/extended-priv/mco_kernel.gotest/extended-priv/mco_security.go
✅ Files skipped from review due to trivial changes (1)
- test/extended-priv/mco_security.go
|
looks good |
| exutil.By("Create a MachineConfig to install the usbguard extension on worker nodes") | ||
| mcExt := NewMachineConfig(oc.AsAdmin(), fmt.Sprintf("test-%s-ext", testID), mcp.GetName()). | ||
| SetMCOTemplate("change-worker-extension-usbguard.yaml") | ||
| defer mcExt.DeleteWithWait() | ||
| mcExt.create() | ||
| logger.Infof("OK!\n") | ||
|
|
||
| exutil.By("Create a MachineConfig to enable the usbguard systemd unit on worker nodes") | ||
| mcEnableName := fmt.Sprintf("test-%s-enable", testID) | ||
| mcEnable := NewMachineConfig(oc.AsAdmin(), mcEnableName, mcp.GetName()) | ||
| mcEnable.SetParams(fmt.Sprintf(`UNITS=[{"enabled": true, "name": "usbguard.service"}]`)) | ||
| defer mcEnable.DeleteWithWait() | ||
| mcEnable.create() | ||
| logger.Infof("OK!\n") |
There was a problem hiding this comment.
nit: For enabling usbguard I think there's no other alternative to apply the change in two steps, one to install the extensions and another one to enable the service, but, to disable it we can do it in one shot saving some time.
There was a problem hiding this comment.
changed the behaviour to delete in one step. Is this the flow you were looking for?
| o.Eventually(newMs.GetIsReady, "20m", "2m").Should(o.BeTrue(), "MachineSet %s is not ready.", newMs.GetName()) | ||
| logger.Infof("OK!\n") | ||
| }) | ||
|
|
There was a problem hiding this comment.
Thank you for the review.
Delete both the extension and enable MachineConfigs in a single oc delete command during cleanup, triggering one pool rollout instead of two. Also revert unintended whitespace change in mco_security.go. Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
|
@HarshwardhanPatil07: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/jira-refresh |
|
Seems good to me. Care to trigger a disruptive suite run? |
Logsharshpat@harshpat-thinkpadp1gen4i:~/Downloads/repos/machine-config-operator$ ./_output/linux/amd64/machine-config-tests-ext run-test "[sig-mco][Suite:openshift/machine-config-operator/longduration][Serial][Disruptive] MCO kernel [PolarionID:88729] Verify USBGuard extension can be installed and enabled via MachineConfig on worker nodes [Disruptive]" I0514 20:53:46.675038 39554 test_context.go:566] The --provider flag is not set. Continuing as if --provider=skeleton had been used. Running Suite: - /home/harshpat/Downloads/repos/machine-config-operator ======================================================================== Random Seed: 1778772226 - will randomize all specs /verified by @HarshwardhanPatil07 |
openshift-ci-robot
added
the
verified
|
@HarshwardhanPatil07: This PR has been marked as verified by DetailsIn response to this:
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. |
|
@pablintino It worked, I ran locally |
4 participants
Adds e2e test that creates two MachineConfigs on worker nodes: one to install the usbguard extension and another to enable the usbguard.service systemd unit, then verifies the service is enabled on all worker nodes.
- What I did
Added a new e2e test (OCP-88729) in test/extended-priv/mco_security.go that verifies USBGuard extension can be installed and enabled via MachineConfig on worker nodes (related to OCPBUGS-78524). The test:
- How to verify it
./_output/linux/amd64/machine-config-tests-ext run-test "[sig-mco][Suite:openshift/machine-config-operator/longduration][Serial][Disruptive] MCO security [PolarionID:88729][OTP] Verify USBGuard extension can be installed and enabled via MachineConfig on worker nodes [Disruptive]"
- Description for the changelog
Add e2e test OCP-88729 to verify USBGuard extension installation and enablement via MachineConfig on worker nodes.
Summary by CodeRabbit