Skip to content

OTA-1165: adm upgrade status: Add Worker Node Status - #1689

Merged
openshift-merge-bot[bot] merged 7 commits into
openshift:masterfrom
DavidHurta:worker-node-status
Mar 6, 2024
Merged

OTA-1165: adm upgrade status: Add Worker Node Status#1689
openshift-merge-bot[bot] merged 7 commits into
openshift:masterfrom
DavidHurta:worker-node-status

Conversation

@DavidHurta

@DavidHurta DavidHurta commented Feb 19, 2024

Copy link
Copy Markdown
Contributor

The goal of this pull request is to display additional information upon executing oc adm upgrade status.

The new additional information being the status of worker pools and nodes.

An example of an output:

An update is in progress for 14m4s: Working towards 4.14.1: 734 of 859 done (85% complete), waiting on machine-config

= Control Plane =
Assessment:      Progressing
Completion:      97%
Duration:        14m4s
Operator Status: 33 Total, 32 Available, 1 Progressing, 0 Degraded

= Worker Upgrade =

= Worker Pool =
Worker Pool:     worker
Assessment:      Progressing
Completion:      0%
Worker Status:   3 Total, 2 Available, 1 Progressing, 3 Outdated, 1 Draining, 0 Excluded, 0 Degraded

Worker Pool Node(s)
NAME                                        ASSESSMENT    PHASE      VERSION   EST    MESSAGE
ip-10-0-4-159.us-east-2.compute.internal    Progressing   Draining   4.14.0    +30m   
ip-10-0-20-162.us-east-2.compute.internal   Outdated      Pending    4.14.0    ?      
ip-10-0-99-40.us-east-2.compute.internal    Outdated      Pending    4.14.0    ?      

= Worker Pool =
Worker Pool:     infra
Assessment:      Progressing
Completion:      0%
Worker Status:   2 Total, 1 Available, 1 Progressing, 2 Outdated, 1 Draining, 0 Excluded, 0 Degraded

Worker Pool Node(s)
NAME                                              ASSESSMENT    PHASE      VERSION   EST    MESSAGE
ip-10-0-4-159-infra.us-east-2.compute.internal    Progressing   Draining   4.14.0    +30m   
ip-10-0-20-162-infra.us-east-2.compute.internal   Outdated      Pending    4.14.0    ?      

= Update Health =
SINCE   LEVEL   IMPACT   MESSAGE
14m4s   Info    None     Upgrade is proceeding well

This pull request references https://issues.redhat.com//browse/OTA-1165

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 19, 2024
@openshift-ci-robot

openshift-ci-robot commented Feb 19, 2024

Copy link
Copy Markdown

@Davoska: This pull request references OTA-1165 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 the "4.16.0" version, but no target version was set.

Details

In 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 openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 19, 2024
@openshift-ci

openshift-ci Bot commented Feb 19, 2024

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@DavidHurta

Copy link
Copy Markdown
Contributor Author

/test all

@DavidHurta
DavidHurta marked this pull request as ready for review February 19, 2024 17:04
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 19, 2024
@openshift-ci
openshift-ci Bot requested review from deads2k and mfojtik February 19, 2024 17:07
@DavidHurta

Copy link
Copy Markdown
Contributor Author

/retest

@DavidHurta

Copy link
Copy Markdown
Contributor Author

/uncc @deads2k @mfojtik

@openshift-ci
openshift-ci Bot removed request for deads2k and mfojtik February 22, 2024 18:46
@DavidHurta

Copy link
Copy Markdown
Contributor Author

Comment thread pkg/cli/admin/upgrade/status/examples/4.15.0-ec2-early.output Outdated
Comment thread pkg/cli/admin/upgrade/status/examples/4.14.1-paused-worker-pool.output Outdated
Comment thread pkg/cli/admin/upgrade/status/status.go Outdated
Comment thread pkg/cli/admin/upgrade/status/workerpool.go Outdated
Comment thread pkg/cli/admin/upgrade/status/status.go Outdated
Comment thread pkg/cli/admin/upgrade/status/workerpool.go Outdated
Comment thread pkg/cli/admin/upgrade/status/workerpool.go
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 28, 2024
Update the old examples to contain information regarding worker pools.
@DavidHurta
DavidHurta force-pushed the worker-node-status branch from fac14d9 to ca178cd Compare March 4, 2024 18:28
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 4, 2024
@openshift-ci-robot

openshift-ci-robot commented Mar 4, 2024

Copy link
Copy Markdown

@Davoska: This pull request references OTA-1165 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 the "4.16.0" version, but no target version was set.

Details

In response to this:

The goal of this pull request is to display additional information upon executing oc adm upgrade status.

The new additional information being the status of worker pools and nodes.

An example of an output:

An update is in progress for 14m4s: Working towards 4.14.1: 734 of 859 done (85% complete), waiting on machine-config

= Control Plane =
Assessment:      Progressing
Completion:      97%
Duration:        14m4s
Operator Status: 33 Total, 32 Available, 1 Progressing, 0 Degraded

= Worker Upgrade =

= Worker Pool =
Worker Pool:     worker
Assessment:      Progressing
Completion:      0%
Worker Status:   3 Total, 2 Available, 1 Progressing, 3 Outdated, 1 Draining, 0 Excluded, 0 Degraded

Worker Pool Node(s)
NAME                                        ASSESSMENT    PHASE      VERSION   EST    MESSAGE
ip-10-0-4-159.us-east-2.compute.internal    Progressing   Draining   4.14.0    +30m   
ip-10-0-20-162.us-east-2.compute.internal   Outdated      Pending    4.14.0    ?      
ip-10-0-99-40.us-east-2.compute.internal    Outdated      Pending    4.14.0    ?      

= Worker Pool =
Worker Pool:     infra
Assessment:      Progressing
Completion:      0%
Worker Status:   2 Total, 1 Available, 1 Progressing, 2 Outdated, 1 Draining, 0 Excluded, 0 Degraded

Worker Pool Node(s)
NAME                                              ASSESSMENT    PHASE      VERSION   EST    MESSAGE
ip-10-0-4-159-infra.us-east-2.compute.internal    Progressing   Draining   4.14.0    +30m   
ip-10-0-20-162-infra.us-east-2.compute.internal   Outdated      Pending    4.14.0    ?      

= Update Health =
SINCE   LEVEL   IMPACT   MESSAGE
14m4s   Info    None     Upgrade is proceeding well

This pull request references https://issues.redhat.com//browse/OTA-1165

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.

@DavidHurta

Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@DavidHurta

Copy link
Copy Markdown
Contributor Author

/retest

@petr-muller

petr-muller commented Mar 5, 2024

Copy link
Copy Markdown
Member

/retitle OTA-1165: adm upgrade status: Add Worker Node Status

@openshift-ci openshift-ci Bot changed the title OTA-1165: Add Worker Node Status OTA-1165: adm upgrade status: Add Worker Node Status Mar 5, 2024
Comment thread pkg/cli/admin/upgrade/status/status.go Outdated
Comment thread pkg/cli/admin/upgrade/status/workerpool.go Outdated
After the control plane is updated, the correct update duration should
still be displayed. Originally, the duration would start over after
a control plane is updated because the LastTransitionTime would change
in the Progressing condition.
The original values were displayed incorrectly and didn't sum to
a correct value.
The needed MCO logic for worker node status is copied over from the
machine-config-operator repository [1]. The required files are copied
over and slightly modified if needed. This is done because vendoring the
original packages from [1] results in massive vendoring changes due to
a small experimental functionality that will be eventually removed and
carried over to a server.

The alternative is to vendor the original packages and introduce the
massive vendoring changes.

[1] https://github.com/openshift/machine-config-operator/tree/11d5151a784c7d4be5255ea41acfbf5092eda592
@DavidHurta
DavidHurta force-pushed the worker-node-status branch from bef8d91 to 092156a Compare March 6, 2024 13:51

@petr-muller petr-muller left a comment

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.

🎉

Really nice job, thanks 👍

@petr-muller

Copy link
Copy Markdown
Member

/lgtm
/approve

@petr-muller

Copy link
Copy Markdown
Member

/cc @ardaguclu

Arda, can we have an /approve for the vendor/* changes? The 092156a commit just runs go mod vendor which vendors more pieces of o/api and o/client-go. No new dependencies are actually added in go.mod.

@openshift-ci
openshift-ci Bot requested a review from ardaguclu March 6, 2024 16:29
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 6, 2024

@ardaguclu ardaguclu left a comment

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.

Vendor changes look good to me;
/approve

@@ -0,0 +1,162 @@
apiVersion: v1

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.

nit: Do we really need to all these examples that are unnecessarily increasing repo size?

@petr-muller petr-muller Mar 6, 2024

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 agree the inputs grew a bit :( I do not expect us to add more testcases but I think we'll eventually need more inputs; I'll make sure we come up with some solution until then, maybe write a script that prunes the mock data from unnecessary members or something similar.

@openshift-ci

openshift-ci Bot commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ardaguclu, Davoska, petr-muller

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 6, 2024
@openshift-ci

openshift-ci Bot commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

@Davoska: 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/test-infra repository. I understand the commands that are listed here.

@openshift-merge-bot
openshift-merge-bot Bot merged commit 45fe5d4 into openshift:master Mar 6, 2024
@openshift-bot

Copy link
Copy Markdown
Contributor

[ART PR BUILD NOTIFIER]

This PR has been included in build openshift-enterprise-cli-container-v4.16.0-202403070215.p0.g45fe5d4.assembly.stream.el8 for distgit openshift-enterprise-cli.
All builds following this will include this PR.

@DavidHurta

Copy link
Copy Markdown
Contributor Author

Thank you, @petr-muller, for all the suggestions, recommendations, and tips on and off GitHub!

It made this pull request much better 🎉

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants