Revert "openstack: Allow to run MCO changes independently of the installer patches" #1040
Conversation
openshift-ci-robot
added
the
size/S
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
@Fedosin that commit (commit 759702e) says it should be reverted once openshift/installer#1959 merges and that installer PR says it needs (commit 759702e). Was that message a mistake? |
|
@kikisdeliveryservice as I understand there is a cyclic dependence between #740 and openshift/installer#1959. So we can't merge neither without breaking the ci. As a fix @mandre suggested to temporary disable template rendering for OpenStack as a part of #740 (759702e), then merge openshift/installer#1959 and finally revert 759702e to enable rendering again. |
|
Correct, 759702e is only useful to keep e2e-openstack green until openshift/installer#1959 merges and can be safely reverted after that. |
Fedosin
force-pushed
the
enable_template_rendering
branch
from
fe7edab to
f73fc74
Compare
Fedosin
changed the title
|
/test e2e-aws-op |
Fedosin
force-pushed
the
enable_template_rendering
branch
from
f73fc74 to
a06a5a6
Compare
|
/test unit |
Fedosin
force-pushed
the
enable_template_rendering
branch
from
a06a5a6 to
b5925b4
Compare
pkg/controller/template/render.go
Outdated
There was a problem hiding this comment.
The problem is that if it merges, can't have green CI on openshift/installer#1959 as this won't render the templates.
In the patch you reverted, I checked for config.ControllerConfigSpec.Infra.Status.PlatformStatus.OpenStack that is set in openshift/installer#1959.
There was a problem hiding this comment.
Something like if config.ControllerConfigSpec.Infra.Status.PlatformStatus.OpenStack == nil && strings.Contains(info.Name(), "openstack") && !strings.Contains(info.Name(), "afterburn-hostname.yaml") { should do it.
…aller patches" This reverts commit 759702e. installer/1959 is merged, so we can enable template rendering again.
Fedosin
force-pushed
the
enable_template_rendering
branch
from
b5925b4 to
38663d3
Compare
Fedosin
changed the title
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/test e2e-aws-op |
|
@Fedosin so @mandre said that this PR might be unneeded now? See here: #1050 (comment) |
|
AHH ok I see, this is now just doing the reverted mentioned in 1050. ok so openshift/installer#1959 has been fully approved but is technically not merged yet, so going to just hold this until it's fully merged so we dont break anything over there (ci is weird and sometimes slow). I'll check in on it today and lift the hold / approve and get this in once that installer PR is purple. /hold |
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
|
/test e2e-aws-upgrade |
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
|
/test e2e-openstack |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Fedosin, runcom 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 |
openshift-ci-robot
added
the
approved
6 participants
This reverts commit 759702e.
openshift/installer#1959 is merged, so we can enable template rendering again.