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

🌱 Skip machines with one WWN, if RAID is desired. #1162

Merged

Conversation

guettli
Copy link
Contributor

@guettli guettli commented Feb 13, 2024

What this PR does / why we need it:

Skip machines with one WWN, if RAID is desired.

Fixes #1122

Special notes for your reviewer:

TODOs:

  • squash commits
  • include documentation
  • add unit tests

@syself-bot syself-bot bot added size/XS Denotes a PR that changes 0-20 lines, ignoring generated files. area/code Changes made in the code directory labels Feb 13, 2024
@guettli guettli force-pushed the tg/dont-pick-machines-with-one-drive-if-raid-is-desired branch from 10785c7 to 8b70456 Compare February 13, 2024 10:03
Copy link
Contributor

@janiskemper janiskemper left a comment

Choose a reason for hiding this comment

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

Is this something that happens often?
This should be documented accordingly. Right now people probably try to do this and wonder why the host does not get chosen.

Should we also include a log or event for this?

@syself-bot syself-bot bot added size/M Denotes a PR that changes 50-200 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-20 lines, ignoring generated files. labels Feb 15, 2024
@guettli guettli force-pushed the tg/dont-pick-machines-with-one-drive-if-raid-is-desired branch from 86e1cdf to 88bd718 Compare February 15, 2024 13:14
@guettli
Copy link
Contributor Author

guettli commented Feb 15, 2024

Is this something that happens often? This should be documented accordingly. Right now people probably try to do this and wonder why the host does not get chosen.

Should we also include a log or event for this?

in 88bd718 i added a "reason", so that users understand why hosts were not picked.

pkg/services/baremetal/baremetal/baremetal.go Show resolved Hide resolved
pkg/services/baremetal/baremetal/baremetal.go Outdated Show resolved Hide resolved
pkg/services/baremetal/baremetal/baremetal.go Outdated Show resolved Hide resolved
pkg/services/baremetal/baremetal/baremetal.go Outdated Show resolved Hide resolved
pkg/services/baremetal/baremetal/baremetal.go Outdated Show resolved Hide resolved
pkg/services/baremetal/baremetal/baremetal.go Outdated Show resolved Hide resolved
pkg/services/baremetal/baremetal/baremetal_test.go Outdated Show resolved Hide resolved
pkg/services/baremetal/baremetal/baremetal_test.go Outdated Show resolved Hide resolved
@guettli guettli force-pushed the tg/dont-pick-machines-with-one-drive-if-raid-is-desired branch from d286906 to e151722 Compare February 29, 2024 09:34
@guettli guettli marked this pull request as draft February 29, 2024 10:26
@guettli guettli marked this pull request as ready for review February 29, 2024 10:43
@guettli guettli marked this pull request as draft February 29, 2024 11:44
@guettli guettli marked this pull request as ready for review February 29, 2024 15:20
@guettli guettli marked this pull request as draft February 29, 2024 16:30
@guettli guettli force-pushed the tg/dont-pick-machines-with-one-drive-if-raid-is-desired branch 2 times, most recently from cdbb078 to de9c340 Compare February 29, 2024 16:38
@guettli guettli marked this pull request as ready for review February 29, 2024 16:38
@guettli
Copy link
Contributor Author

guettli commented Mar 1, 2024

@janiskemper please review

@janiskemper
Copy link
Contributor

did you do more than fixing the test? If not, I'll approve

@guettli
Copy link
Contributor Author

guettli commented Mar 1, 2024

did you do more than fixing the test? If not, I'll approve

tiny changes got done:

I increased the log-level so that the message is actually visible:

s.scope.Info("No available host found. Requeuing.", "reason", reason)

if there is no available host, then the error message is more explicit now:

	if availableHosts == 0 {
		return nil, nil, fmt.Sprintf("No available host found. %d hosts exist.",
			len(hosts.Items)), nil
	}

@syself-bot syself-bot bot added size/L Denotes a PR that changes 200-800 lines, ignoring generated files. and removed size/M Denotes a PR that changes 50-200 lines, ignoring generated files. labels Mar 1, 2024
@guettli guettli force-pushed the tg/dont-pick-machines-with-one-drive-if-raid-is-desired branch from a7eb356 to 5246350 Compare March 4, 2024 08:45
Additionally a better error message gets returned, if no matching
HetznerBaremetalHost was found.
@guettli guettli force-pushed the tg/dont-pick-machines-with-one-drive-if-raid-is-desired branch from 5246350 to 6fdeb96 Compare March 4, 2024 08:45
@guettli guettli merged commit cb8d5cf into main Mar 4, 2024
9 checks passed
@guettli guettli deleted the tg/dont-pick-machines-with-one-drive-if-raid-is-desired branch March 4, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/code Changes made in the code directory size/L Denotes a PR that changes 200-800 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Controller should not pick hbmh with one disk, if swraid=true
3 participants