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: missing command #102

Merged

Conversation

burnsjared0415
Copy link
Contributor

Summary

Command $cluster = Get-VCFCluster | Where-Object { $_.id -eq ($workloadDomain.clusters.id) } was removed on accident, also updated a couple variables

Type

  • Bugfix
  • Enhancement or Feature
  • Code Style or Formatting
  • Documentation
  • Refactoring
  • Chore
  • Other
    Please describe:

Breaking Changes?

  • Yes, there are breaking changes.
  • No, there are no breaking changes.

Test and Documentation

  • Tests have been completed.
  • Documentation has been added or updated.

Issue References

Closes #101

Additional Information

@github-actions github-actions bot added the needs-review Needs Review label Apr 4, 2024
SampleScripts/PowerManagement-ManagementDomain.ps1 Outdated Show resolved Hide resolved
SampleScripts/PowerManagement-ManagementDomain.ps1 Outdated Show resolved Hide resolved
SampleScripts/PowerManagement-ManagementDomain.ps1 Outdated Show resolved Hide resolved
SampleScripts/PowerManagement-ManagementDomain.ps1 Outdated Show resolved Hide resolved
SampleScripts/PowerManagement-ManagementDomain.ps1 Outdated Show resolved Hide resolved
@burnsjared0415 burnsjared0415 changed the title bug:Missing command fix:Missing command Apr 4, 2024
@tenthirtyam tenthirtyam marked this pull request as ready for review April 4, 2024 13:24
@tenthirtyam tenthirtyam requested a review from a team as a code owner April 4, 2024 13:24
@tenthirtyam tenthirtyam changed the title fix:Missing command fix: missing command Apr 4, 2024
@burnsjared0415 burnsjared0415 force-pushed the bugfix/issues-with-missing-command branch from bb3f405 to 9d2e8df Compare April 4, 2024 19:29
@tenthirtyam tenthirtyam requested a review from garlicNova April 4, 2024 19:34
@tenthirtyam tenthirtyam force-pushed the bugfix/issues-with-missing-command branch from 9d2e8df to f389490 Compare April 4, 2024 20:52
@tenthirtyam tenthirtyam self-requested a review April 4, 2024 20:52
Copy link
Contributor

@garlicNova garlicNova left a comment

Choose a reason for hiding this comment

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

There seemed to be validation issue with this section. by changing $clustername to $clustername_extra functional breaks for prior cmdlet calls at line 205, 228, 250, 251, 257, 267 and 330.

SampleScripts/PowerManagement-ManagementDomain.ps1 Outdated Show resolved Hide resolved
SampleScripts/PowerManagement-ManagementDomain.ps1 Outdated Show resolved Hide resolved
SampleScripts/PowerManagement-ManagementDomain.ps1 Outdated Show resolved Hide resolved
@garlicNova
Copy link
Contributor

I am unable to validate the commit due to other issues as well.
line: 196 foreach ($clusterid in $mgmtClusterIds) when the cluster in which sfo-m01-vc01 vm exists is higher than other cluster in this list, the function will shutdown sfo-m01-vc01 vm before other clusters in the domain shutdown down.
line 349: if i sort the cluster list to have the main cluster be the last one on the list, the script ends at line 349 and the startup json is not created, therefore i am unable to powerup the environment through the script.
Please make the above changes and re-run the module E2E based on step 6 in https://vmware.github.io/powershell-module-for-vmware-cloud-foundation-power-management/documentation/#usage-examples

@tenthirtyam tenthirtyam added this to the v1.4.2 milestone Apr 15, 2024
@tenthirtyam tenthirtyam added the bug Bug label Apr 15, 2024
@burnsjared0415
Copy link
Contributor Author

line: 196 foreach ($clusterid in $mgmtClusterIds) when the cluster in which sfo-m01-vc01 vm exists is higher than other cluster in this list, the function will shutdown sfo-m01-vc01 vm before other clusters in the domain shutdown down. i am not sure understand this comment. I am really not sure what the issues are, i can't replicate these issues

@garlicNova
Copy link
Contributor

garlicNova commented May 2, 2024

line: 196 foreach ($clusterid in $mgmtClusterIds) when the cluster in which sfo-m01-vc01 vm exists is higher than other cluster in this list, the function will shutdown sfo-m01-vc01 vm before other clusters in the domain shutdown down. i am not sure understand this comment. I am really not sure what the issues are, i can't replicate these issues

ah i found the issue, line 199 $isDefault is undefined, so it caused the issue where mgmt domain cluster is shutting down before non Default clusters. Please see below suggestions

@tenthirtyam tenthirtyam requested a review from joisika July 15, 2024 16:47
Command `$cluster = Get-VCFCluster | Where-Object { $_.id -eq ($workloadDomain.clusters.id) }` was removed on accident, also updated a couple variables

Signed-off-by: Jared Burns <jared.burns@broadcom.com>
@tenthirtyam tenthirtyam force-pushed the bugfix/issues-with-missing-command branch from 9e0a69f to 48045f9 Compare July 15, 2024 16:53
@tenthirtyam tenthirtyam modified the milestones: v1.4.2, v1.4.1 Jul 15, 2024
@tenthirtyam tenthirtyam removed the needs-review Needs Review label Jul 16, 2024
@tenthirtyam
Copy link
Collaborator

Merging this one after discussions with @joisika. He will review the current state and initiate some in-depth testing for the next release.

@tenthirtyam tenthirtyam merged commit ef59f92 into vmware:develop Jul 16, 2024
1 check passed
@vmware vmware locked and limited conversation to collaborators Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$cluster.name is empty, PowerManagement-ManagementDomain.ps1 errors out
3 participants