Skip to content

OSDOCS13124 Add version information to the output oc get machineconfignodes/Add MCP column to MCN output#86943

Merged
mburke5678 merged 1 commit intoopenshift:mainfrom
mburke5678:mco-machineconfignode-output
Jan 29, 2025
Merged

OSDOCS13124 Add version information to the output oc get machineconfignodes/Add MCP column to MCN output#86943
mburke5678 merged 1 commit intoopenshift:mainfrom
mburke5678:mco-machineconfignode-output

Conversation

@mburke5678
Copy link
Copy Markdown
Contributor

@mburke5678 mburke5678 commented Jan 13, 2025

https://issues.redhat.com/browse/OSDOCS-13124

Previews:
Machine configuration -> Machine configuration overview -> About checking machine config node status -- Updated module.
Machine configuration -> Machine configuration overview -> Checking machine config node status -- New module

QE review:

  • QE has approved this change.

@mburke5678 mburke5678 added this to the Planned for 4.18 GA milestone Jan 13, 2025
@openshift-ci openshift-ci Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 13, 2025
@ocpdocs-previewbot
Copy link
Copy Markdown

ocpdocs-previewbot commented Jan 13, 2025

🤖 Tue Jan 28 18:30:57 - Prow CI generated the docs preview:

https://86943--ocpdocs-pr.netlify.app/openshift-enterprise/latest/machine_configuration/index.html

@openshift-ci openshift-ci Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 13, 2025
Comment thread modules/checking-mco-node-status.adoc Outdated
@mburke5678 mburke5678 changed the title Add version information to the output oc get machineconfignodes Add version information to the output oc get machineconfignodes/Add MCP column to MCN output Jan 15, 2025
@mburke5678 mburke5678 changed the title Add version information to the output oc get machineconfignodes/Add MCP column to MCN output OSDOCS13124 Add version information to the output oc get machineconfignodes/Add MCP column to MCN output Jan 15, 2025
@mburke5678
Copy link
Copy Markdown
Contributor Author

@isabella-janssen @djoshy Can you PTAL?

Comment thread modules/checking-mco-node-status.adoc Outdated

[source,terminal]
----
$ oc get machineconfignodes
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe this should be oc get machineconfig

spec:
featureSet: TechPreviewNoUpgrade <1>
----
<1> Enables the required `SigstoreImageVerification` feature.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Should this reference the MachineConfigNodes feature?

Copy link
Copy Markdown
Contributor Author

@mburke5678 mburke5678 Jan 16, 2025

Choose a reason for hiding this comment

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

@isabella-janssen That is the name of the feature gate that is enabled in order to enable the MachineConfigNodes feature. I can change it. Or, better, remove the whole line.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think that's accurate, this feature should be tied to the MachineConfigNodes feature gate.

Stepping back a bit, a feature set selects a group of feature gates. So when you enable TechPreviewNoUpgrade, all the feature gates defined under it get enabled. At the time of writing, the tech preview featureset includes both SigStoreImageVerification and MachineConfigNodes features, among other tech preview features.

Could you point to where you found this link b/w SigStore and MCN? Might be something we need to fix if it is incorrectly listed somewhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry. I think this was a bad copy-and-paste. I am working on a TP feature related to sigstore. I removed the reference, as we don't use it often in the docs.

Comment thread modules/checking-mco-node-status.adoc Outdated
* *Update Executed*. The MCO cordons and drains the node, and applies the new machine config to the node files and operating system, as needed. It contains the following sub-phases:
** *Cordoned*
** *Drained*
** *UpdateExecutedAppliedFilesAndOS*
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For consistency with the rest of the list, should this be AppliedFilesAndOS?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

err, this might be my lack of understanding - but where is the mapping for phases/subphases from? I'm having a hard time following 😓

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@djoshy Most of it came from MCO-821: Introduce MachineConfigNode types. I'm not sure the subphases are really needed.

@isabella-janssen
Copy link
Copy Markdown
Member

Just a few questions/suggestions but otherwise looks good to me. Thanks 🙂

@mburke5678
Copy link
Copy Markdown
Contributor Author

@sergiordlr PTAL

The node update process consists of the following phases and subphases that are tracked by the machine config node custom resource:

* *Update Prepared*. The MCO stops the configuration drift monitoring and verifies that the newly-created machine config can be applied to a node.
** *UpdateCompatible*
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we get a description about "UpdateCompatible" field? I'm not sure about its meaning.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Explained later in the section. I added text to the introductory sentence stating this.


As the update moves through these phases, you can query the `MachineConfigNode` custom resource, which reports one of the following conditions for each phase:

* `True`. The phase is complete on that node or the MCO has started that phase on that node.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We could include a note explaining that once the nodes is fully updated all values are reset.

@mburke5678 mburke5678 added the peer-review-needed Signifies that the peer review team needs to review this PR label Jan 28, 2025
@mburke5678 mburke5678 force-pushed the mco-machineconfignode-output branch from e453ba5 to dc1cbbe Compare January 28, 2025 18:20
@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented Jan 28, 2025

@mburke5678: all tests passed!

Full PR test history. Your PR dashboard.

Details

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.

@GroceryBoyJr
Copy link
Copy Markdown
Contributor

/label peer-review-in-progress
/remove-label peer-review-needed
/assign GroceryBoyJr

@openshift-ci openshift-ci Bot added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Jan 28, 2025
Copy link
Copy Markdown
Contributor

@GroceryBoyJr GroceryBoyJr left a comment

Choose a reason for hiding this comment

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

LGTM

@GroceryBoyJr
Copy link
Copy Markdown
Contributor

/label peer-review-done
/remove-label peer-review-in-progress
/unassign GroceryBoyJr

@openshift-ci openshift-ci Bot added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Jan 28, 2025
@mburke5678 mburke5678 merged commit f1be8c0 into openshift:main Jan 29, 2025
@mburke5678 mburke5678 deleted the mco-machineconfignode-output branch January 29, 2025 13:03
@mburke5678
Copy link
Copy Markdown
Contributor Author

/cherrypick enterprise-4.18

@openshift-cherrypick-robot
Copy link
Copy Markdown

@mburke5678: new pull request created: #87757

Details

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.18 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.

8 participants