Skip to content

Conversation

@thunderboltsid
Copy link
Contributor

Update controllerconfig CRD and relevant switch statements
in pkg to handle Nutanix platform.

@kikisdeliveryservice
Copy link
Contributor

/test verify
/test unit

@thunderboltsid thunderboltsid force-pushed the nutanix-platform branch 2 times, most recently from 6fbcb47 to 32abbf2 Compare February 14, 2022 17:13
@thunderboltsid
Copy link
Contributor Author

@thunderboltsid
Copy link
Contributor Author

thunderboltsid commented Feb 14, 2022

ci/prow/unit failures https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift_machine-config-operator/2942/pull-ci-openshift-machine-config-operator-master-unit/1493272225430638592 started happening when API dependency got updated.

Trying to narrow down the offending commit. The unit tests don't fail when bumping the API dependency to 58db72a40994aaa03e79fd9d7ec791987a5fb1dc. However, when bumping it to the the commit next in chronological order de2aae00abd3f89af713a9c718eeb0ad1a12905c they start failing.

Failing tests:

--- FAIL: TestFeatureGateDrift (0.40s)
    --- FAIL: TestFeatureGateDrift/AWS (0.13s)
    --- FAIL: TestFeatureGateDrift/None (0.13s)
    --- FAIL: TestFeatureGateDrift/unrecognized (0.13s)

--- FAIL: TestFeaturesDefault (0.79s)
    --- FAIL: TestFeaturesDefault/AWS (0.26s)
    --- FAIL: TestFeaturesDefault/None (0.26s)
    --- FAIL: TestFeaturesDefault/unrecognized (0.26s)

--- FAIL: TestBootstrapFeaturesDefault (0.18s)
    --- FAIL: TestBootstrapFeaturesDefault/AWS (0.06s)
    --- FAIL: TestBootstrapFeaturesDefault/None (0.06s)
    --- FAIL: TestBootstrapFeaturesDefault/unrecognized (0.06s)

@thunderboltsid
Copy link
Contributor Author

/retest

Copy link
Contributor

@kikisdeliveryservice kikisdeliveryservice left a comment

Choose a reason for hiding this comment

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

would you mind squashing your newest commit(abca3c7e6251c0a8e86ab898b9619ec70118acca) into the prev one(e270c7d49fa9a3c5c108dfed3df0ef1652773cad)

@kikisdeliveryservice
Copy link
Contributor

adding a hold to let us get the api dependency sorted out between the 2 prs. once https://github.com/openshift/machine-config-operator/pull/2949/files lands shortly we can update this one.

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2022
@thunderboltsid
Copy link
Contributor Author

would you mind squashing your newest commit(abca3c7) into the prev one(e270c7d)

Sure.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2022
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 24, 2022
@kikisdeliveryservice kikisdeliveryservice removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 24, 2022
Copy link
Contributor

@kikisdeliveryservice kikisdeliveryservice left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @thunderboltsid

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 24, 2022
@kikisdeliveryservice
Copy link
Contributor

Since they'll be approvers for this platform: @yanhua121 @adiantum can you PTAL

@thunderboltsid
Copy link
Contributor Author

/retest

@thunderboltsid
Copy link
Contributor Author

/retest-required

1 similar comment
@thunderboltsid
Copy link
Contributor Author

/retest-required

@adiantum
Copy link
Member

Since they'll be approvers for this platform: @yanhua121 @adiantum can you PTAL

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 25, 2022

@adiantum: changing LGTM is restricted to collaborators

Details

In response to this:

Since they'll be approvers for this platform: @yanhua121 @adiantum can you PTAL

/lgtm

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-required

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

9 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@thunderboltsid
Copy link
Contributor Author

/retest-required

@openshift-bot
Copy link
Contributor

/retest-required

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

3 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@kikisdeliveryservice
Copy link
Contributor

There are some IAM issues that are causing some of these jobs to fail. I'll go investigate and put a hold on this for now to avoid us all getting spammed with retests and failures.

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 22, 2022
@kikisdeliveryservice
Copy link
Contributor

/skip

@kikisdeliveryservice
Copy link
Contributor

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 22, 2022
@kikisdeliveryservice
Copy link
Contributor

/test e2e-aws

@openshift-bot
Copy link
Contributor

/retest-required

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 23, 2022

@thunderboltsid: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-workers-rhel7 891ee12edb7f6087816b0a73ad4a4adfe1620d9a link false /test e2e-aws-workers-rhel7
ci/prow/e2e-aws-workers-rhel8 d2b2442 link false /test e2e-aws-workers-rhel8
ci/prow/e2e-aws-disruptive d2b2442 link false /test e2e-aws-disruptive
ci/prow/e2e-aws-single-node d2b2442 link false /test e2e-aws-single-node
ci/prow/e2e-aws-upgrade-single-node d2b2442 link false /test e2e-aws-upgrade-single-node
ci/prow/e2e-vsphere-upgrade d2b2442 link false /test e2e-vsphere-upgrade

Full PR test history. Your PR dashboard.

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.

@openshift-bot
Copy link
Contributor

/retest-required

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-merge-robot openshift-merge-robot merged commit fce8f7c into openshift:master Mar 23, 2022
wking added a commit to wking/cincinnati-graph-data that referenced this pull request May 3, 2023
Discussion in [1] has some details, but trying to unpack "all on-prem
IPI except baremetal IPI" into specifics, [2] is in an on-prem
directory configuring keepalived, and it switches on
onPremPlatformAPIServerInternalIP for enabled vs. disabled.
onPremPlatformAPIServerInternalIP is true (enabling the keepalived
configuration) for:

* BareMetal (4.10 [3] and 4.11 [4])
* oVirt (4.10 [3] and 4.11 [4])
* OpenStack (4.10 [3] and 4.11 [4])
* VSphere (4.10 [3] and 4.11 [4]),
* KubeVirt (4.10 [3], dropped in 4.11 [4,5])
* Nutanix (new in 4.11 [4,6,7]).

Before 4.11, ENABLE_UNICAST was conditional on
onPremPlatformKeepalivedEnableUnicast [8], but since 4.11, it has
always been 'yes' [9].  The platforms that were unicast on 4.10's
onPremPlatformKeepalivedEnableUnicast were BareMetal and KubeVirt
[10].

Putting this all together, AWS and other platforms that don't match
the onPremPlatformAPIServerInternalIP logic aren't impacted, because
they don't enable the keepalived configuration.  BareMetal is not
impacted by 4.10-to-4.11 updates, because any to-unicast transition
issues will already have been resolved by 4.10.  Remaining
onPremPlatformAPIServerInternalIP platforms which occur in both 4.10
and 4.11 are interested, and I match them here.

Generated by writing the 4.11.0 declaration by hand, and then copying
out to other 4.11 releases with:

  $ curl -s 'https://api.openshift.com/api/upgrades_info/graph?channel=candidate-4.11' | jq -r '.nodes[].version' | grep '^4[.]11[.]' | grep -v '^4[.]11[.]0$' | while read V; do sed "s/4[.]11[.]0/${V}/g" blocked-edges/4.11.0-KeepalivedMulticastSkew.yaml > "blocked-edges/${V}-KeepalivedMulticastSkew.yaml"; done
  $ git add blocked-edges/4.11.*KeepalivedMulticastSkew.yaml

[1]: https://issues.redhat.com/browse/OPNET-296
[2]: https://github.com/openshift/machine-config-operator/blame/8fa0b7e8903226b3cfb76e6c6f49409cfc0dd0e7/templates/common/on-prem/files/keepalived.yaml#L2
[3]: https://github.com/openshift/machine-config-operator/blob/afb47c916680dd5870e48e5c9cf819f59e12ff4d/pkg/operator/render.go#L282-L294
[4]: https://github.com/openshift/machine-config-operator/blob/8fa0b7e8903226b3cfb76e6c6f49409cfc0dd0e7/pkg/operator/render.go#L282-L294
[5]: openshift/machine-config-operator#3084
[6]: openshift/machine-config-operator#2942
[7]: https://docs.openshift.com/container-platform/4.11/release_notes/ocp-4-11-release-notes.html#ocp-4-11-nutanix
[8]: https://github.com/openshift/machine-config-operator/blob/afb47c916680dd5870e48e5c9cf819f59e12ff4d/templates/common/on-prem/files/keepalived.yaml#L155-L156
[9]: openshift/machine-config-operator@84d0bae#diff-c4a27bc4c14847dd581f495e992f67cf49b430644e8f113aabfa879de076564dL156
[10]: https://github.com/openshift/machine-config-operator/blob/afb47c916680dd5870e48e5c9cf819f59e12ff4d/pkg/operator/render.go#L249-L250
wking added a commit to wking/cincinnati-graph-data that referenced this pull request May 4, 2023
Discussion in [1] has some details, but trying to unpack "all on-prem
IPI except baremetal IPI" into specifics, [2] is in an on-prem
directory configuring keepalived, and it switches on
onPremPlatformAPIServerInternalIP for enabled vs. disabled.
onPremPlatformAPIServerInternalIP is true (enabling the keepalived
configuration) for:

* BareMetal (4.10 [3] and 4.11 [4])
* oVirt (4.10 [3] and 4.11 [4])
* OpenStack (4.10 [3] and 4.11 [4])
* VSphere (4.10 [3] and 4.11 [4]),
* KubeVirt (4.10 [3], dropped in 4.11 [4,5])
* Nutanix (new in 4.11 [4,6,7]).

Before 4.11, ENABLE_UNICAST was conditional on
onPremPlatformKeepalivedEnableUnicast [8], but since 4.11, it has
always been 'yes' [9].  The platforms that were unicast on 4.10's
onPremPlatformKeepalivedEnableUnicast were BareMetal and KubeVirt
[10].

Putting this all together, AWS and other platforms that don't match
the onPremPlatformAPIServerInternalIP logic aren't impacted, because
they don't enable the keepalived configuration.  BareMetal is not
impacted by 4.10-to-4.11 updates, because any to-unicast transition
issues will already have been resolved by 4.10.  Remaining
onPremPlatformAPIServerInternalIP platforms which occur in both 4.10
and 4.11 are interested, and I match them here.

Generated by writing the 4.11.0 declaration by hand, and then copying
out to other 4.11 releases with:

  $ curl -s 'https://api.openshift.com/api/upgrades_info/graph?channel=candidate-4.11' | jq -r '.nodes[].version' | grep '^4[.]11[.]' | grep -v '^4[.]11[.]0$' | while read V; do sed "s/4[.]11[.]0/${V}/g" blocked-edges/4.11.0-KeepalivedMulticastSkew.yaml > "blocked-edges/${V}-KeepalivedMulticastSkew.yaml"; done
  $ git add blocked-edges/4.11.*KeepalivedMulticastSkew.yaml

[1]: https://issues.redhat.com/browse/OPNET-296
[2]: https://github.com/openshift/machine-config-operator/blame/8fa0b7e8903226b3cfb76e6c6f49409cfc0dd0e7/templates/common/on-prem/files/keepalived.yaml#L2
[3]: https://github.com/openshift/machine-config-operator/blob/afb47c916680dd5870e48e5c9cf819f59e12ff4d/pkg/operator/render.go#L282-L294
[4]: https://github.com/openshift/machine-config-operator/blob/8fa0b7e8903226b3cfb76e6c6f49409cfc0dd0e7/pkg/operator/render.go#L282-L294
[5]: openshift/machine-config-operator#3084
[6]: openshift/machine-config-operator#2942
[7]: https://docs.openshift.com/container-platform/4.11/release_notes/ocp-4-11-release-notes.html#ocp-4-11-nutanix
[8]: https://github.com/openshift/machine-config-operator/blob/afb47c916680dd5870e48e5c9cf819f59e12ff4d/templates/common/on-prem/files/keepalived.yaml#L155-L156
[9]: openshift/machine-config-operator@84d0bae#diff-c4a27bc4c14847dd581f495e992f67cf49b430644e8f113aabfa879de076564dL156
[10]: https://github.com/openshift/machine-config-operator/blob/afb47c916680dd5870e48e5c9cf819f59e12ff4d/pkg/operator/render.go#L249-L250
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. 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