-
Notifications
You must be signed in to change notification settings - Fork 247
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
✨ Add Golang-based deploy-cli #1669
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
0b80030
to
74088ff
Compare
74088ff
to
3471248
Compare
6395f97
to
e5632a8
Compare
/cc @dtantsur @elfosardo @zaneb I think you would like to check this out also |
@Rozzii: GitHub didn't allow me to request PR reviews from the following users: I, to, out, check, this, also, would, like, you, think. Note that only metal3-io members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
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. |
/test metal3-bmo-e2e-test-pull |
/cc @adilGhaffarDev @lentzi90 @Rozzii @kashifest |
9cd23ab
to
9fe6024
Compare
9fe6024
to
ce222e0
Compare
675eb64
to
e8722d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good just some small things I noticed 😄
I has open comments etc as well, but in general I don't want this in BMO 0.7 yet, IMO better merge it only after we've cut the release, so it can have a cycle to be tested, polished and integrated. Let me know if you disagree. |
I'm not sure what you mean by "cut the release"? IMO waiting for a whole release to merge this is a waste of time. It's passed all CI tests, so if we don't merge it and let people use it, how can we further test it then? We still keep Edit: Sorry now I think I got your point. If it just means we will wait till after 0.7 release, then we merge this, I wouldn't mind. Anw, as I said, it wouldn't hurt to have it in 0.7 either. |
fc29d0f
to
6cd48bd
Compare
Yes, I meant 0.7, but if its not replacing deploy.sh, and deploy.sh is still the documented way, then it doesn't matter so much. I was rather thinking that we make this one replace deploy.sh after we've cut 0.7, so it gets tested in CI for a release cycle as primary tool, and there wouldn't be two tools to maintain. How does that sound? |
Yep that would be great! We will have to do that change in dev-env though, so doesn't matter if we merge this PR or not. |
Yes I'm aware of that. However we need to maintain this in 0.7 if we merge it before cutting release, so I think we are agreeing on merging only after 0.7? |
Yeah let's merge after 0.7 then |
6cd48bd
to
10bb8f5
Compare
10bb8f5
to
60ea3a5
Compare
Signed-off-by: Max Rantil <max.rantil@est.tech> Signed-off-by: Huy Mai <huy.mai@est.tech>
60ea3a5
to
74f768e
Compare
/unhold We have BMO 0.8 released. |
/unhold |
What this PR does / why we need it:
This package does the jobs that are currently handled by
deploy.sh
. It will help installing BMO, Ironic with suitable configuration.This PR has been successfully tested here: metal3-io/metal3-dev-env#1447
Co-author: @maxrantil