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

Fix printing of duplicate machine pool information #555

Conversation

ThirumlaDevi
Copy link
Contributor

@ThirumlaDevi ThirumlaDevi commented Sep 25, 2023

fixes OCM-3680

Files changed

  • removed the duplicate default worker card printer from cmd/ocm/list/machinepool/cmd.go file
  • Added new test suite to check this functionality

@ThirumlaDevi ThirumlaDevi force-pushed the OCM_3680_duplicate_default_machine_pool_info branch 2 times, most recently from 5b8d258 to c931d24 Compare September 27, 2023 10:58
Copy link
Collaborator

@jhernand jhernand left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @ThirumlaDevi.

@miguelsorianod
Copy link

miguelsorianod commented Sep 27, 2023

The changes look good to me 👍 .

Some notes: this changes the behavior of the list machinepools command compared to previous OCM CLI versions:

  • Now when listing a cluster that has only the machinepool automatically created on cluster creation it shows the machine pool id of that cluster, which is named worker at the moment of writing this, instead of an entry with the default name. This means no default entry is showed anymore when listing a cluster that has only the machinepool automatically created on cluster creation
  • There is no concept of a "default machinepool" anymore on the list machinepools command. The machinepools command simply outputs the list of machinepools IDs retrieved by Cluster Service. One of the machinepools created in the cluster can have the ID default by itself and therefore it would be shown as the result of the list machinepools output, although it doesn't mean it is a "default machinepool", it is just the ID.

I understand this behavior change was discussed and accepted by the appropriate parties as discussed. If that were not to be the case please double-check with whoever is required. Thanks.

@ThirumlaDevi ThirumlaDevi force-pushed the OCM_3680_duplicate_default_machine_pool_info branch from c931d24 to f0fadd8 Compare September 27, 2023 11:29
@gdbranco gdbranco merged commit 1884874 into openshift-online:main Sep 27, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants