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

Proposal of adding metal3host operator. #268

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sailorvii
Copy link

What this PR does / why we need it:
In some scenarios, customers like to acquire an empty bare-metal host or a group of hosts.
Furthermore, users hope that host could pre-install the customized application.
It's a pure bare-metal management platform without guest Kubernetes.

Which issue(s) this PR fixes (optional, in fixes #(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #metal3-io/cluster-api-provider-metal3#649

@metal3-io-bot
Copy link
Contributor

Hi @sailorvii. Thanks for your PR.

I'm waiting for a metal3-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@metal3-io-bot metal3-io-bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 7, 2022
@dtantsur
Copy link
Member

/ok-to-test

From a quick review: which component will host the new controller?

@metal3-io-bot metal3-io-bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 11, 2022
@sailorvii
Copy link
Author

/ok-to-test

From a quick review: which component will host the new controller?

I thought it's good to new a repository. This component is based on the bare-metal operator but parallels with capm3.

@metal3-io-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@metal3-io-bot metal3-io-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 18, 2022
@metal3-io-bot
Copy link
Contributor

Stale issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle stale.

/close

@metal3-io-bot
Copy link
Contributor

@metal3-io-bot: Closed this PR.

In response to this:

Stale issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle stale.

/close

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.

@Rozzii
Copy link
Member

Rozzii commented Jan 18, 2023

/reopen

@metal3-io-bot metal3-io-bot reopened this Jan 18, 2023
@metal3-io-bot
Copy link
Contributor

@Rozzii: Reopened this PR.

In response to this:

/reopen

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.

@Rozzii
Copy link
Member

Rozzii commented Jan 18, 2023

@sailorvii is it still something that you would be interested to push forward?


The bare-metal host controller and the ironic are not newly developed.

Metal3Host is a virtual instance. Users could define it to require a bare-metal host. In the definition, the user could specify the image to install, could set the selector to choose a specific bare-metal host, and could define the boot data to automatically start their own applications.
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we actually want to have Metal3 in the names. The difference between Metal3Host and BareMetalHost is not going to be obvious. It's pity that the word "Deployment" has a different meaning...

Copy link
Member

Choose a reason for hiding this comment

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

I even wonder if we need an object at all. Cannot we only use BareMetalHost's and a new BareMetalDeployment (renamed from Metal3HostDeployment)? BareMetalHosts already have all the deployment information inside of them.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your review.
The difference between metal3Host and baremetalHost is that metal3Host is a virtual host. When the user requires a host, a metal3Host instance will be created. The metal3Host instance has some features the user defines, such as the operating system, the user name/password, the bootup application, etc. After the instance is created, it will choose one suitable baremetalHost.

About the name, I'm not clear which is best.

Thanks & Regards,
Wei

Copy link
Author

Choose a reason for hiding this comment

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

Another side, if one baremetalHost is down, the metal3Host could choose another baremetalHost. We also could label the baremetalHost, so users could define their requirements such as they need a strong CPU host. Then the metal3Host could choose a host by the label. The metal3Host represents the users' requirement, the baremetal3Host represents the physical machine. I think it's more flexible.

Copy link
Member

Choose a reason for hiding this comment

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

I see so the host is handled like a workload, like a pod.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I can agree with a new object, but I cannot agree with calling it Metal3Host. BareMetalHost is also a Metal3 host technically. Could be BareMetalInstance, ProvisionedBareMetal, BareMetalProvisioning... Having a Metal3 prefix is not terribly helpful, definitely does not solve the potential confusion.

Copy link
Member

Choose a reason for hiding this comment

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


#### metal3host

'''golang
Copy link
Member

Choose a reason for hiding this comment

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

Formatting is off here, should be

```go

```

without extra indentation.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. I'll refine it.

@dtantsur
Copy link
Member

dtantsur commented Mar 1, 2023

/remove-lifecycle stale

@metal3-io-bot metal3-io-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 1, 2023

Metal3HostDeployment will define a group of metal3Hosts with the same configuration. It could define the number of replicas. It could scale up/down. We even could auto-scale by some rules after the monitor function is leveraged.

The data operator will set up the boot time configuration & routine. It includes three parts: the userData, the metaData, and the networkData. The userData includes some user-specified data such as installing a package, or running a command like "mkdir /test". The metaData defines some host-specific data such as hostname. The networkData includes the network configuration. The data is bound with the baremetal host. For example, the networkData defines a NIC with an IP, the NIC name is host related. So a dataTemplate should be pre-defined, it will be bound to the bare-metal host when a metal3host is defined and associated with a bare-metal host.
Copy link
Member

Choose a reason for hiding this comment

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

It is not clear to me why do you need a new pair of data and dataTemplate CR, wouldn't it be easier to just re-use the existing metal3Data and metal3DataTemplates?

You would also use the new data and the dataTemplate for the same config process as the existing metal3Data and metal3DataTemplate is used for.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, almost the same with metal3Data. One reason I want to create a new CR is, the metal3Data is bound to the metal3Machine, so it's not easy to modify if any change is required. Another is that the metal3Host and data could be an independent application(new a repo).
Maybe I'm wrong about this.

Copy link
Member

Choose a reason for hiding this comment

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

What's the mechanism of linking machines to metal3Data? If it's ownerReference, we may be able to link different CR's there.


The bare-metal host controller and the ironic are not newly developed.

Metal3Host is a virtual instance. Users could define it to require a bare-metal host. In the definition, the user could specify the image to install, could set the selector to choose a specific bare-metal host, and could define the boot data to automatically start their own applications.
Copy link
Member

Choose a reason for hiding this comment

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

I see so the host is handled like a workload, like a pod.

@metal3-io-bot
Copy link
Contributor

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues will close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@metal3-io-bot metal3-io-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 20, 2023
@metal3-io-bot
Copy link
Contributor

Stale issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle stale.

/close

@metal3-io-bot
Copy link
Contributor

@metal3-io-bot: Closed this PR.

In response to this:

Stale issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle stale.

/close

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.

@Rozzii
Copy link
Member

Rozzii commented Sep 20, 2023

/life-cycle-frozen
I think this proposal is fundamentally a good idea.

@Rozzii Rozzii reopened this Sep 20, 2023
@metal3-io metal3-io deleted a comment from metal3-io-bot Sep 20, 2023
@Rozzii Rozzii added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. kind/feature Categorizes issue or PR as related to a new feature. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 20, 2023
@sailorvii
Copy link
Author

/life-cycle-frozen I think this proposal is fundamentally a good idea.

Thank you for your recognition.

@metal3-io-bot
Copy link
Contributor

@sailorvii: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
spellcheck bf6f35a link true /test spellcheck
shellcheck bf6f35a link true /test shellcheck

Full PR test history. Your PR dashboard.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: BMO on hold / blocked
Development

Successfully merging this pull request may close these issues.

4 participants