Skip to content

Conversation

@bcrochet
Copy link
Member

@bcrochet bcrochet commented Nov 5, 2020

The VSphere UPI install does not fully populate PlatformStatus. When we deduped
the templates, we didn't account for this being nil. This should render these
templates empty when that PlatformStatus is nil.

- What I did

- How to verify it

- Description for the changelog

@openshift-ci-robot openshift-ci-robot added the bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. label Nov 5, 2020
@openshift-ci-robot
Copy link
Contributor

@bcrochet: This pull request references Bugzilla bug 1895099, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.7.0) matches configured target release for branch (4.7.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
Details

In response to this:

Bug 1895099: Fix VSphere UPI not populating PlatformStatus

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.

@openshift-ci-robot openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Nov 5, 2020
@patrickdillon
Copy link
Contributor

cc @jcpowermac

@patrickdillon
Copy link
Contributor

Not sure if these are setup:
/test e2e-vsphere
/test e2e-vsphere-upi

@patrickdillon
Copy link
Contributor

/test e2e-vsphere
/test e2e-vsphere-upi

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we need to replace this with the simpler conditional used in other templates? {{ if (onPremPlatformAPIServerInternalIP .) -}}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no reference to the PlatformStatus in that file. Are you saying you don't want it at all?

@patrickdillon
Copy link
Contributor

The VSphere UPI install does not fully populate PlatformStatus. When we deduped
the templates, we didn't account for this being nil. This should render these
templates empty when that PlatformStatus is nil.
@jcpowermac
Copy link
Contributor

/test e2e-vsphere-upi

@jcpowermac
Copy link
Contributor

/test e2e-vsphere

@patrickdillon
Copy link
Contributor

/test e2e-vsphere
/test e2e-vsphere-upi

@patrickdillon
Copy link
Contributor

CI is consistently failing with Failed pulling builder image not sure what's going on there. I do wonder if the issue fixed by this pr could be causing part of the problem, as 4.7 images are not being promoted.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 8, 2020
@bcrochet
Copy link
Member Author

bcrochet commented Nov 9, 2020

/retest

@jcpowermac
Copy link
Contributor

/test e2e-vsphere-upi

@jcpowermac
Copy link
Contributor

/test e2e-vsphere

@jcpowermac
Copy link
Contributor

MCO is still configured for template vs. multistep at least for UPI.

@jcpowermac
Copy link
Contributor

/test e2e-vsphere-upi

@bcrochet
Copy link
Member Author

This may not pass until #2209 is merged. It might need a rebase after.

@jcpowermac
Copy link
Contributor

@bcrochet looking at job results the installation succeeded the tests failed. I think this could move forward as-is.

@patrickdillon wdyt?

@bcrochet
Copy link
Member Author

bcrochet commented Nov 10, 2020

@bcrochet looking at job results the installation succeeded the tests failed. I think this could move forward as-is.

@patrickdillon wdyt?

If you're ok with the order, it doesn't matter to me. This patch would probably allow #2209 to pass, or vice versa. This one is probably failing because of the problem fixed in the #2209.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

16 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@jcpowermac
Copy link
Contributor

Any chance we can get an override for gcp-op? This PR is needed for vSphere UPI to install and CI to pass.

@yuqi-zhang
Copy link
Contributor

Waiting for some other CI troubles to get resolved (namely AWS which is blocking the GCP fix). Will override by EoD if those are still blocking

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@yuqi-zhang
Copy link
Contributor

Ok given the AWS still slowly recovering, and the fact that this has passed all the other tests in the past + it should not affect cloud platforms, I will override this.

/override ci/prow/e2e-gcp-op

@openshift-ci-robot
Copy link
Contributor

@yuqi-zhang: Overrode contexts on behalf of yuqi-zhang: ci/prow/e2e-gcp-op

Details

In response to this:

Ok given the AWS still slowly recovering, and the fact that this has passed all the other tests in the past + it should not affect cloud platforms, I will override this.

/override ci/prow/e2e-gcp-op

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.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit e02ed51 into openshift:master Nov 18, 2020
@openshift-ci-robot
Copy link
Contributor

@bcrochet: All pull requests linked via external trackers have merged:

Bugzilla bug 1895099 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1895099: Fix VSphere UPI not populating PlatformStatus

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

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants