-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Added SINGLE_NODE_CLUSTER="true" configuration to ipi install #15187
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
Added SINGLE_NODE_CLUSTER="true" configuration to ipi install #15187
Conversation
b6f527b to
f3c610c
Compare
|
/test pj-rehearse |
f3c610c to
72e89bb
Compare
|
/test pj-rehearse |
72e89bb to
f5a3736
Compare
|
/test pj-rehearse |
|
/test ? |
|
@omertuc: The following commands are available to trigger jobs:
Use
DetailsIn 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 pull-ci-openshift-installer-master-e2e-sno-mult |
|
@omertuc: The specified target(s) for
Use
DetailsIn 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. |
f5a3736 to
0b47112
Compare
|
/test ci/rehearse/openshift/installer/master/e2e-sno-mult |
|
@omertuc: The specified target(s) for
Use
DetailsIn 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 pj-rehearse ? |
|
/test pj-rehearse |
0b47112 to
7049794
Compare
|
/test pj-rehearse |
|
@omertuc: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
7049794 to
384f274
Compare
|
/test pj-rehearse |
384f274 to
c99d0c5
Compare
|
/test pj-rehearse |
1e6ecf3 to
a16b97b
Compare
|
/test pj-rehearse |
a16b97b to
5b0ae32
Compare
|
/test pj-rehearse |
ci-operator/step-registry/ipi/install/install/ipi-install-install-ref.yaml
Outdated
Show resolved
Hide resolved
ci-operator/step-registry/ipi/install/install/ipi-install-install-ref.yaml
Outdated
Show resolved
Hide resolved
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.
Do we have yq in this image?
Then it might become prettier
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.
Or maybe install it as you do with pyyaml
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.
I considered that, I think Python is a better fit here than yq - there's a lot of logic here to support various edge-cases caused by the variety of platforms that generate the install-config.yaml before this step - sometimes they don't define controlPlane or don't define compute - I'm not sure how simple that is to handle with yq.
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.
yq can either add or update fields, depending on if they already exist on the document
Give it a try, it will make the code more explicit and without the python implementation detail. Also it will allow extensions in the future without changing the Python code
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.
Doesn't seem like there's a simple way to install yq here - the version in pip is rather old and with a different syntax (even less similar to the familiar jq), I don't think relying on that version is a good idea - they've already deprecated V3 (aug 21) - the version in pip is V2. I'll keep it Python for now unless you can come up with a way to install it. Or maybe it'll be a good idea to avoid it altogether, seeing that it's rather unstable and they keep deprecating the syntax
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.
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.
As noted - in your own specific configuration step, you are free to use any container image you need, which can include yq.
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.
Eventually i've decided to avoid yq altogether due to:
Or maybe it'll be a good idea to avoid it altogether, seeing that it's rather unstable and they keep deprecating the syntax
And in this case, I'm modifying an existing installer configuration step - it's not mine, I prefer to avoid modifications to its Dockerfile.
ci-operator/step-registry/ipi/install/install/ipi-install-install-commands.sh
Outdated
Show resolved
Hide resolved
ci-operator/config/openshift/installer/openshift-installer-master.yaml
Outdated
Show resolved
Hide resolved
5b0ae32 to
6b1cc1e
Compare
|
/test pj-rehearse |
stevekuznetsov
left a comment
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.
Take a look at the various configuration steps and workflows that already exist. SDN for instance - there is the sdn-conf step for SDN specific configuration and the openshift-e2e-aws-sdn-multi workflow that uses it. It's a composition approach. This PR changes the install step for everyone, which is not ideal. Create a new workflow for your novel approach and compose it with a new configuration step.
6b1cc1e to
803bafa
Compare
Followed SDN's footsteps as you recommended, updated PR |
|
/test pj-rehearse |
|
@omertuc: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: omertuc, stevekuznetsov The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse |
|
@omertuc: Updated the following 5 configmaps:
DetailsIn 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. |
Added e2e-aws-single-node workflow to test single node on AWS