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

Move ironic into cluster #68

Merged
merged 1 commit into from
Oct 7, 2019

Conversation

stbenjam
Copy link
Member

@stbenjam stbenjam commented Sep 30, 2019

This deploys the Ironic containers in the cluster itself, rather than
the provisioning host. The provisioning host still maintains an httpd
server for hosting a cache of the IPA images, as well as any additional
images the user wants to provision with (by default we populate it with
CentOS).

Goes with metal3-io/baremetal-operator#309

fixes #41

@metal3-io-bot metal3-io-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 30, 2019
@hardys
Copy link
Member

hardys commented Sep 30, 2019

Thoughts?

Unless the downloading needs to be cluster-hosted as well it seems potentially simpler to download directly onto the host, then either run httpd natively, or run a container with httpd that mounts the host directory with the images?

03_launch_mgmt_cluster.sh Outdated Show resolved Hide resolved
@stbenjam
Copy link
Member Author

stbenjam commented Sep 30, 2019

Thoughts?

Unless the downloading needs to be cluster-hosted as well it seems potentially simpler to download directly onto the host, then either run httpd natively, or run a container with httpd that mounts the host directory with the images?

@hardys We want the IPA images to be downloaded in the cluster though, right? I made the ipa-image-downloader an init container. I feel like it's kind of nice that all those bits are all hosted in one place.

Could we run two different http servers for metal3-dev-env? The one on the provisioning host would be for a user to dump any provisioning images they want in (and we prepopulate it with CentOS).

@hardys
Copy link
Member

hardys commented Sep 30, 2019

@hardys We want the IPA images to be downloaded in the cluster though, right?

Well, it's not essential but yes - note we can use CACHEURL to point the ipa downloader container at the host, so we don't download the RDO tarball every run which will save some time/bandwidth.

@stbenjam
Copy link
Member Author

Ah good point about CACHEURL. I've put the httpd container back on the provisioning host, so we can grab centos there. IPA is still downloaded from within the cluster. I think this looks closer to how we did things with OpenShift.

Should be ready for a look, along with metal3-io/baremetal-operator#309

@russellb
Copy link
Member

/test-centos-integration

01_prepare_host.sh Outdated Show resolved Hide resolved
@stbenjam
Copy link
Member Author

/test-centos-integration

01_prepare_host.sh Show resolved Hide resolved
01_prepare_host.sh Show resolved Hide resolved
01_prepare_host.sh Outdated Show resolved Hide resolved
01_prepare_host.sh Outdated Show resolved Hide resolved
01_prepare_host.sh Outdated Show resolved Hide resolved
01_prepare_host.sh Outdated Show resolved Hide resolved
@metal3-io-bot metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 1, 2019
@stbenjam
Copy link
Member Author

stbenjam commented Oct 1, 2019

/test-centos-integration

@stbenjam
Copy link
Member Author

stbenjam commented Oct 1, 2019

/test-centos-integration

1 similar comment
@stbenjam
Copy link
Member Author

stbenjam commented Oct 1, 2019

/test-centos-integration

@maelk
Copy link
Member

maelk commented Oct 2, 2019

To fix the interface issue, something similar to #71 can be done

01_prepare_host.sh Outdated Show resolved Hide resolved
01_prepare_host.sh Show resolved Hide resolved
@stbenjam
Copy link
Member Author

stbenjam commented Oct 2, 2019

/test-centos-integration

1 similar comment
@russellb
Copy link
Member

russellb commented Oct 2, 2019

/test-centos-integration

Makefile Outdated Show resolved Hide resolved
@stbenjam
Copy link
Member Author

stbenjam commented Oct 2, 2019

/test-centos-integration

@stbenjam
Copy link
Member Author

stbenjam commented Oct 2, 2019

/test-integration

@metal3-io-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stbenjam

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

The pull request process is described here

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

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 3, 2019
@stbenjam
Copy link
Member Author

stbenjam commented Oct 3, 2019

/test-centos-integration

@stbenjam
Copy link
Member Author

stbenjam commented Oct 3, 2019

/test-integration

@stbenjam
Copy link
Member Author

stbenjam commented Oct 3, 2019

Last CI run passed: https://jenkins.nordix.org/job/airship_metal3io_metal3_dev_env_integration_test_ubuntu/34/

https://jenkins.nordix.org/job/airship_metal3io_metal3_dev_env_integration_test_centos/27/

I've squashed everything, and moved the code back to hitting metal3-io/baremetal-operator instead of my fork.

Copy link
Member

@maelk maelk left a comment

Choose a reason for hiding this comment

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

Some checks could be added on the ironic setup, for the configmap and secret. For the deployment there is no need as this will be checked using the check_k8s_entity since the name does not change.

04_verify.sh Show resolved Hide resolved
01_prepare_host.sh Outdated Show resolved Hide resolved
This deploys the Ironic containers in the cluster itself, rather than
the provisioning host. The provisioning host still maintains an httpd
server for hosting a cache of the IPA images, as well as any additional
images the user wants to provision with (by default we populate it with
CentOS).
@russellb
Copy link
Member

russellb commented Oct 4, 2019

merged the baremetal-operator PR, so let's run CI on this one more time ...

/test-centos-integration

@russellb
Copy link
Member

russellb commented Oct 4, 2019

@stbenjam Can you prepare a docs PR against metal3-io/metal3-io.github.io that reflects the changes to how you would talk to the Ironic API? See the "try it" page contents.

@stbenjam
Copy link
Member Author

stbenjam commented Oct 5, 2019

/test-centos-integration

@russellb
Copy link
Member

russellb commented Oct 5, 2019

/hold

Just seeing if prow is listening now ...

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 5, 2019
@russellb
Copy link
Member

russellb commented Oct 5, 2019

Yay

/hold cancel

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 5, 2019
@stbenjam
Copy link
Member Author

stbenjam commented Oct 5, 2019

/test-integration

1 similar comment
@stbenjam
Copy link
Member Author

stbenjam commented Oct 5, 2019

/test-integration

@maelk
Copy link
Member

maelk commented Oct 7, 2019

Can we merge this ? Metal3-dev-env is broken without this.

@stbenjam
Copy link
Member Author

stbenjam commented Oct 7, 2019

@maelk Sure, I think so, anything else we can address in follow-ups. If you do /lgtm tide should merge this automatically

@maelk
Copy link
Member

maelk commented Oct 7, 2019

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 7, 2019
@metal3-io-bot metal3-io-bot merged commit b060bff into metal3-io:master Oct 7, 2019
@stbenjam stbenjam deleted the ironic-in-cluster branch October 7, 2019 11:47
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. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move Ironic into the cluster
6 participants