OCPBUGS-99695: Add TC 88940- Apply password only if changes exist - #6328
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
WalkthroughAdds a disruptive test verifying password updates when the password hash changes, while SSH-key-only updates do not reconfigure the password or trigger drain/reboot operations. ChangesPassword update validation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_password.go`:
- Around line 448-449: Remove node.GetName() and any other node-identifying
values from the assertion messages associated with IgnoreEventsBeforeNow and the
analogous checks at the referenced locations. Keep each failure message
descriptive without exposing hostnames or other internal node identifiers.
🪄 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: Pro Plus
Run ID: 0c28a1c4-830c-4b67-afe3-2de8b29c5008
📒 Files selected for processing (1)
test/extended-priv/mco_password.go
|
/test security |
2ca5a9f to
bd1f966
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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_password.go`:
- Around line 462-467: Update the password-configuration verification around
mc.create() to capture the pwdConfiguredLogMsg count from
node.GetMCDaemonLogs("") before applying the MachineConfig, then retrieve the
logs afterward and assert the count increased. Replace the current
initialPasswordCount > 0 check while preserving the existing error handling and
node-specific assertion context.
🪄 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: Pro Plus
Run ID: d493ad0d-e989-4019-ae1c-09cd39ebfc10
📒 Files selected for processing (1)
test/extended-priv/mco_password.go
| exutil.By("Check MCD logs to verify 'Password has been configured' is logged") | ||
| mcdLogs, err := node.GetMCDaemonLogs("") | ||
| o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MCD logs for node %s", node.GetName()) | ||
| initialPasswordCount := strings.Count(mcdLogs, pwdConfiguredLogMsg) | ||
| o.Expect(initialPasswordCount).To(o.BeNumerically(">", 0), | ||
| "Expected '%s' in MCD logs after initial password application on node %s", pwdConfiguredLogMsg, node.GetName()) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Capture the pre-apply log count.
initialPasswordCount > 0 passes when the node already has this message from prior activity, so it does not prove the initial MachineConfig emitted it. Snapshot the count before mc.create() and compare against that baseline.
Proposed fix
+ mcdLogsBeforeApply, err := node.GetMCDaemonLogs("")
+ o.Expect(err).NotTo(o.HaveOccurred(), "Error getting MCD logs")
+ passwordCountBeforeApply := strings.Count(mcdLogsBeforeApply, pwdConfiguredLogMsg)
+
exutil.By("Apply MC with passwordHash for core user")
...
- o.Expect(initialPasswordCount).To(o.BeNumerically(">", 0),
+ o.Expect(initialPasswordCount).To(o.BeNumerically(">", passwordCountBeforeApply),Based on the PR objective to verify initial password-configuration logging.
🤖 Prompt for 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.
In `@test/extended-priv/mco_password.go` around lines 462 - 467, Update the
password-configuration verification around mc.create() to capture the
pwdConfiguredLogMsg count from node.GetMCDaemonLogs("") before applying the
MachineConfig, then retrieve the logs afterward and assert the count increased.
Replace the current initialPasswordCount > 0 check while preserving the existing
error handling and node-specific assertion context.
|
@ptalgulk01: 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. |
|
/lgtm |
|
Pipeline controller notification No second-stage tests were triggered for this PR. This can happen when:
Use |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ptalgulk01, sergiordlr The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
openshift-ci-robot
added
the
verified
|
@ptalgulk01: 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. |
|
/label acknowledge-critical-fixes-only |
openshift-ci
Bot
added
the
acknowledge-critical-fixes-only
ptalgulk01
changed the title
openshift-ci-robot
added
jira/severity-important
|
@ptalgulk01: This pull request references Jira Issue OCPBUGS-83830, 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. |
ptalgulk01
changed the title
|
@ptalgulk01: This pull request references Jira Issue OCPBUGS-99695, 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. |
openshift-ci-robot
removed
the
jira/severity-important
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@ptalgulk01: This pull request references Jira Issue OCPBUGS-99695, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
|
/cherry-pick release-4.22 |
|
@ptalgulk01: once the present PR merges, I will cherry-pick it on top of 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 kubernetes-sigs/prow repository. |
|
/jira refresh |
openshift-ci-robot
added
the
jira/severity-low
|
@ptalgulk01: This pull request references Jira Issue OCPBUGS-99695, which is valid. 3 validation(s) were run on this bug
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. |
|
@ptalgulk01: Jira Issue Verification Checks: Jira Issue OCPBUGS-99695 Jira Issue OCPBUGS-99695 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
@ptalgulk01: new pull request created: #6339 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 kubernetes-sigs/prow repository. |
|
Fix included in release 5.0.0-0.nightly-2026-07-25-032712 |
5 participants
Added e2e test OCP-88940 to verify that MCD applies password usermod only when the password hash actually changes (OCPBUGS-83830).
The test covers:
Run on a cluster:
./_output/linux/amd64/machine-config-tests-ext run --test "PolarionID:88940"
Description for the changelog
Add e2e test OCP-88940 validating MCD only runs password usermod when the hash changes, not on unrelated passwd section updates like SSH key additions.
Summary by CodeRabbit