Skip to content

Replace networkmanager scripts with podman wrappers#1521

Closed
vrutkovs wants to merge 1 commit intoopenshift:masterfrom
vrutkovs:nm-scripts-containerized
Closed

Replace networkmanager scripts with podman wrappers#1521
vrutkovs wants to merge 1 commit intoopenshift:masterfrom
vrutkovs:nm-scripts-containerized

Conversation

@vrutkovs
Copy link
Copy Markdown
Contributor

@vrutkovs vrutkovs commented Feb 28, 2020

- What I did
Replaced python scripts for NetworkManager with podman wrappers

- How to verify it
Run vSphere / oVirt / Openstack IPI installs - workers should join cluster as expected

- Description for the changelog
NM helper scripts are running from podman containers

See openshift/baremetal-runtimecfg#46

@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 28, 2020
@openshift-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vrutkovs
To complete the pull request process, please assign yuqi-zhang
You can assign the PR to them by writing /assign @yuqi-zhang in a comment when ready.

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

Details 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

@LorbusChris
Copy link
Copy Markdown
Contributor

Do we have OpenStack e2e here?
/test e2e-openstack

@LorbusChris
Copy link
Copy Markdown
Contributor

/cc @cgwalters @runcom

@vrutkovs
Copy link
Copy Markdown
Contributor Author

/hold

worker nodes didn't join :/

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 28, 2020
@LorbusChris LorbusChris self-assigned this Feb 28, 2020
@LorbusChris
Copy link
Copy Markdown
Contributor

does the container maybe need to be privileged in this case?

@vrutkovs
Copy link
Copy Markdown
Contributor Author

unprivildged container should work too.

Its odd that workers didn't join as NotReady nodes - lets see if that is consistent

/retest

@LorbusChris
Copy link
Copy Markdown
Contributor

/retest

@vrutkovs
Copy link
Copy Markdown
Contributor Author

ffs, so non-virtual-ip script is python2 and nodeip-finder one is python3-only. As a result it crashes when UBI7's python2 is used

Python may not be included in base OS, so the scripts should be running from the container, instead of being placed on the host
@vrutkovs vrutkovs force-pushed the nm-scripts-containerized branch from 9814088 to 148b29d Compare February 28, 2020 16:55
vrutkovs pushed a commit to vrutkovs/machine-config-operator that referenced this pull request Feb 28, 2020
Avoid running python scripts on host and use `podman run --net=host` instead

Cherrypicked to master as openshift#1521
@kikisdeliveryservice
Copy link
Copy Markdown
Contributor

As MCO doesn't own these templates, we will ask that approval is given by the baremental/openstack folks as a requirement to merge.

/assign @Fedosin @celebdor @mandre @yboaron

@kikisdeliveryservice
Copy link
Copy Markdown
Contributor

/assign @rgolangh

@rgolangh
Copy link
Copy Markdown
Contributor

rgolangh commented Mar 3, 2020

/test e2e-ovirt

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@vrutkovs: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-gcp-op 148b29d link /test e2e-gcp-op
ci/prow/e2e-aws 148b29d link /test e2e-aws
ci/prow/e2e-aws-scaleup-rhel7 148b29d link /test e2e-aws-scaleup-rhel7
ci/prow/e2e-gcp-upgrade 148b29d link /test e2e-gcp-upgrade
ci/prow/images 148b29d link /test images
ci/prow/e2e-openstack 9814088c730d6da821368a5394b7cf1364cdd6e6 link /test e2e-openstack
ci/prow/e2e-ovirt 148b29d link /test e2e-ovirt

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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.

@LorbusChris
Copy link
Copy Markdown
Contributor

This won't work because the UBI7 containers don't have Python 3 included, as noted above by @vrutkovs. We also can't switch over to UBI8 for the baremetal-runtimecfg container base because that won't run on RHEL7, which is still supported as worker nodes in OCP.

If I'm not mistaken @celebdor is working on translating the scripts to Golang, which would we be the preferred solution here anyway.

This is also tracked in #1430

I believe this PR can thus be closed?

@LorbusChris
Copy link
Copy Markdown
Contributor

I don't see this as applicable to master at this time.
/close

@openshift-ci-robot
Copy link
Copy Markdown
Contributor

@LorbusChris: Closed this PR.

Details

In response to this:

I don't see this as applicable to master at this time.
/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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants