Add egress-router-cni init container #815
Conversation
danielmellado
changed the title
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
Pull-request updated, HEAD is now 914a2e0 |
danielmellado
force-pushed
the
add_multus_egress_router
branch
from
87e8e50
to
914a2e0
Compare
|
/assign rcarrillocruz |
|
/assign @dougbtv |
|
Pull-request updated, HEAD is now 03c5af0 |
danielmellado
force-pushed
the
add_multus_egress_router
branch
from
914a2e0
to
03c5af0
Compare
|
Pull-request updated, HEAD is now 03c5af0 |
There was a problem hiding this comment.
@danielmellado don't forget the image-references updates too here @ https://github.com/openshift/cluster-network-operator/blob/master/manifests/image-references
bindata/network/multus/multus.yaml
Outdated
| @@ -133,6 +133,24 @@ spec: | |||
| value: "/usr/src/multus-cni/rhel8/bin/" | |||
| - name: DEFAULT_SOURCE_DIRECTORY | |||
| value: "/usr/src/multus-cni/bin/" | |||
| - name: egress-router-binary-copy | |||
| image: "quay.io/dmellado/egress-router-test:latest" | |||
There was a problem hiding this comment.
Make sure this winds up using the templated {{.ImageNameHere}} when you're ready to take this out of WIP
danielmellado
force-pushed
the
add_multus_egress_router
branch
from
03c5af0
to
bd414f1
Compare
danielmellado
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
danielmellado
force-pushed
the
add_multus_egress_router
branch
2 times, most recently
from
d22bf7d
to
f27d616
Compare
|
I had been meaning to add https://issues.redhat.com/browse/SDN-695 to the 4.7 epics, which I have belatedly done now. We really need a cleaner system for installing CNI plugins. I had wanted to say that solving this problem would block the addition of any new initcontainers in CNO in 4.7, though ideally I would have say that before you wrote this PR... |
danielmellado
force-pushed
the
add_multus_egress_router
branch
from
f27d616
to
93bce9f
Compare
Given that I'm basically relying on multus' daemonset here, I was hoping that we could bypass that for now. |
|
/retest |
|
Pull-request updated, HEAD is now 531c010 |
danielmellado
force-pushed
the
add_multus_egress_router
branch
from
59ab337
to
531c010
Compare
|
Pull-request updated, HEAD is now f9b07b4 |
danielmellado
force-pushed
the
add_multus_egress_router
branch
from
531c010
to
f9b07b4
Compare
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
/hold cancel - All prerequisites are merged now. |
|
/retest |
This commit adds a new init container for the deployment of the egress-router-cni plugin to the hosts.
danielmellado
force-pushed
the
add_multus_egress_router
branch
from
b23cce2
to
87d4207
Compare
|
/approve |
openshift-ci-robot
added
the
approved
|
/test e2e-aws-sdn-multi |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
| @@ -55,6 +55,8 @@ spec: | |||
| value: "5000" | |||
| - name: OVN_CONTROLLER_INACTIVITY_PROBE | |||
| value: "30000" | |||
| - name: EGRESS_ROUTER_CNI_IMAGE | |||
| value: "quay.io/openshift/origin-egress-router-cni:latest" | |||
There was a problem hiding this comment.
Not very fond of the idea of having it pointing to latest, but every other image is doing it so I guess that's not terrible
|
/lgtm note to self: change adds the new egress-router-container image, similar to existing ones. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
6 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
/refresh |
|
@danielmellado: The following tests failed, say
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. |
|
/lgtm |
|
@danielmellado: you cannot LGTM your own PR. 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danielmellado, rcarrillocruz, tssurya 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 |
|
/retest |
|
@danielmellado: The following tests failed, say
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. |
|
/retest |
This commit adds a new init container for the deployment of the
egress-router-cni plugin to the hosts.