Bug 1936443: Revert "baremetal: send full ignition to masters" #4722

hardys · 2021-03-08T14:00:32Z

This doesn't work for IPI baremetal deployments driven via hive,
because there are firewall rules that prevent access to the
bootstrap MCS from the pod running the installer.

This was implemented in:
#4427

But we ran into problems making the same approach work for
worker machines ref:
#4456

We're now looking at other approaches to resolve the
network-config requirements driving that work, so
switching back to the pointer config for masters seems
reasonable, particularly given this issue discovered for
hive deployments.

Conflicts:
pkg/tfvars/baremetal/baremetal.go

This reverts commit 98dc381.

hardys · 2021-03-08T14:01:13Z

WIP pending testing and more details via a bz (since we'll likely want to backport this to 4.7)

/cc @stbenjam @kirankt

hardys · 2021-03-08T14:01:30Z

/label platform/baremetal

hardys · 2021-03-08T14:46:52Z

https://bugzilla.redhat.com/show_bug.cgi?id=1936443

This doesn't work for IPI baremetal deployments driven via hive, because there are firewall rules that prevent access to the bootstrap MCS from the pod running the installer. This was implemented in: openshift#4427 But we ran into problems making the same approach work for worker machines ref: openshift#4456 We're now looking at other approaches to resolve the network-config requirements driving that work, so switching back to the pointer config for masters seems reasonable, particularly given this issue discovered for hive deployments. Conflicts: pkg/tfvars/baremetal/baremetal.go This reverts commit 98dc381.

hardys · 2021-03-09T12:10:09Z

/retitle Bug 1936443: Revert "baremetal: send full ignition to masters"

openshift-ci-robot · 2021-03-09T12:10:28Z

@hardys: This pull request references Bugzilla bug 1936443, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.8.0) matches configured target release for branch (4.8.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla (augol@redhat.com), skipping review request.

In response to this:

Bug 1936443: Revert "baremetal: send full ignition to masters"

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.

hardys · 2021-03-09T12:19:16Z

/retest

stbenjam · 2021-03-09T12:26:47Z

/approve
/assign @kirankt

@kirankt Could you please have a look and leave a lgtm if it's ok? Thanks!

kirankt

/lgtm

openshift-ci-robot · 2021-03-10T13:09:18Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kirankt, stbenjam

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:

~~data/data/baremetal/OWNERS~~ [stbenjam]
~~pkg/tfvars/baremetal/OWNERS~~ [stbenjam]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2021-03-10T13:13:44Z

/retest

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

openshift-bot · 2021-03-10T13:27:05Z

/retest

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

openshift-bot · 2021-03-10T14:57:43Z

/retest

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

openshift-bot · 2021-03-10T16:47:16Z

/retest

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

openshift-bot · 2021-03-10T17:07:43Z

/retest

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

openshift-bot · 2021-03-10T18:16:41Z

/retest

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

openshift-bot · 2021-03-10T22:58:48Z

/retest

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

openshift-bot · 2021-03-10T23:11:58Z

/retest

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

openshift-bot · 2021-03-10T23:50:45Z

/retest

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

openshift-bot · 2021-03-11T05:49:46Z

/retest

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

openshift-ci · 2021-03-11T07:19:30Z

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

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-workers-rhel7	`35c5089`	link	`/test e2e-aws-workers-rhel7`

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.

openshift-bot · 2021-03-11T07:22:17Z

/retest

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

openshift-ci-robot · 2021-03-11T08:13:45Z

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

openshift/installer#4722

Bugzilla bug 1936443 has been moved to the MODIFIED state.

In response to this:

Bug 1936443: Revert "baremetal: send full ignition to masters"

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.

hardys · 2021-03-11T11:15:46Z

/cherry-pick release-4.7

openshift-cherrypick-robot · 2021-03-11T11:16:36Z

@hardys: #4722 failed to apply on top of branch "release-4.7":

Applying: Revert "baremetal: send full ignition to masters"
Using index info to reconstruct a base tree...
M	data/data/baremetal/main.tf
M	data/data/baremetal/variables-baremetal.tf
M	pkg/tfvars/baremetal/baremetal.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/tfvars/baremetal/baremetal.go
CONFLICT (content): Merge conflict in pkg/tfvars/baremetal/baremetal.go
Auto-merging data/data/baremetal/variables-baremetal.tf
Auto-merging data/data/baremetal/main.tf
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Revert "baremetal: send full ignition to masters"
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-4.7

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.

blkingrh · 2021-03-17T13:30:02Z

The ACM team would like to test with this when it is available. Is this expected in 4.7.3? It doesn't look like the PR merged correctly ^^^^. Is there a nightly build we can test with now, or is it still to come?

stbenjam · 2021-03-17T14:06:07Z

@blkingrh It's in 4.8 nightlies now, not sure which z-Stream it will make in 4.7.

@hardys @kirankt Would one of you be able to manually cherry-pick this to release-4.7?

kirankt · 2021-03-17T16:15:05Z

@blkingrh It's in 4.8 nightlies now, not sure which z-Stream it will make in 4.7.

@hardys @kirankt Would one of you be able to manually cherry-pick this to release-4.7?

@stbenjam I'll do the manual cherry pick. Thanks.

blkingrh · 2021-03-17T16:17:08Z

Thank you for the updates, @kirankt @stbenjam . Could you add a comment here when it is in a 4.7 nightly so we can know which one to pick up?

blkingrh · 2021-03-23T14:50:04Z

@kirankt @stbenjam Just checking back to see if this is in a 4.7 nightly or if there is a rough ETA on that? Thanks.

kirankt · 2021-03-23T15:51:05Z

@blkingrh Its not merged yet. I'm waiting on 4.8 revert to get verified by QA before the 4.7 backport can move along... I'll update you when it shows up in the nightlies... Thanks.

blkingrh · 2021-04-06T12:27:13Z

Just checking in to see if there is an ETA on when this will be available in 4.7? Thanks.

anthonyfister · 2021-04-13T14:58:13Z

Tagging so I can get updates as well.
@kirankt There are several ACM zStreams that are dependent upon this fix getting back ported to 4.7.

kirankt · 2021-04-13T15:30:28Z

This 4.8 bug still need to be verified by QA before 4.7 gets any attention. Unfortunately this out of my control, but I'll make sure to bug the QA folks to look into the issue.

blkingrh · 2021-04-28T11:18:49Z

@kirankt any new news on the QA verification? Thanks.

kirankt · 2021-04-29T14:01:25Z

@kirankt any new news on the QA verification? Thanks.

@blkingrh AFAIK, its still in the same state. I've reached out to QA to see when they can start verifying this bug, so that the backports can make progress... Thanks.

blkingrh · 2021-05-06T19:58:34Z

@kirankt We have a major customer who we believe is going to be looking for this support in the near future. Can you put us in touch with the QA lead so that we can understand whether or not this is in plan and what the timeline is?

kirankt · 2021-05-06T21:12:41Z

@kirankt We have a major customer who we believe is going to be looking for this support in the near future. Can you put us in touch with the QA lead so that we can understand whether or not this is in plan and what the timeline is?

Hi @blkingrh . The QA contact is in the bugzilla link. Let me update it and make it Urgent, especially if there is a customer waiting for it...

Edit. BZ link: https://bugzilla.redhat.com/show_bug.cgi?id=1936443

evelinec · 2021-05-11T14:19:41Z

The BZ bug was verified by Alexander Chuzhoy:
I verified https://bugzilla.redhat.com/show_bug.cgi?id=1936443
:bugzilla: #1936443 [VERIFIED] Hive based OCP IPI baremetal installation fails to connect to API VIP port 22623 – high/:warning:urgent in Installer for 4.8/4.8.0

Can we move forward with the 4.7 fix?

kirankt · 2021-05-11T15:12:30Z

The 4.7 bug is now in verified state and should move forward with the appropriate labels.

blkingrh · 2021-05-14T14:47:33Z

Thank you @kirankt . When do you anticipate that we can pick this up in a 4.7 build?

evelinec · 2021-05-19T13:12:10Z

Can you let us know if the 4.7 fix is in any build that we can verify?

kirankt · 2021-05-19T13:52:17Z

@evelinec , @blkingrh Please pose the questions regarding when the patch gets into a release in the appropriate bugzillas. The right people will see and respond to it. On github, as a developer, I can only work on the fix and see it off to completion. I honestly don't know the workings of the release process and github isn't the place to pose questions related to the release. Please comment/reply on the BZ.

evelinec · 2021-05-19T15:07:16Z

Thank you @kirankt.

Just fyi for others, here's the 4.7 BZ, https://bugzilla.redhat.com/show_bug.cgi?id=1940275. I requested some info there.

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 8, 2021

openshift-ci-robot requested review from kirankt and stbenjam March 8, 2021 14:01

openshift-ci-robot added the platform/baremetal IPI bare metal hosts platform label Mar 8, 2021

openshift-ci-robot requested review from celebdor and dhellmann March 8, 2021 14:01

hardys force-pushed the full_ignition_revert branch from 47ea1ee to 35c5089 Compare March 8, 2021 14:48

openshift-ci-robot changed the title ~~WIP: Revert "baremetal: send full ignition to masters"~~ Bug 1936443: Revert "baremetal: send full ignition to masters" Mar 9, 2021

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 9, 2021

openshift-ci-robot added 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. labels Mar 9, 2021

openshift-ci-robot assigned kirankt Mar 9, 2021

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 9, 2021

kirankt approved these changes Mar 10, 2021

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2021

openshift-merge-robot merged commit 78c0350 into openshift:master Mar 11, 2021

kirankt mentioned this pull request Mar 18, 2021

Bug 1940275: Revert "baremetal: send full ignition to masters" #4767

Merged

Bug 1936443: Revert "baremetal: send full ignition to masters" #4722

Bug 1936443: Revert "baremetal: send full ignition to masters" #4722

Conversation

hardys commented Mar 8, 2021

hardys commented Mar 8, 2021

hardys commented Mar 8, 2021

hardys commented Mar 8, 2021

hardys commented Mar 9, 2021

openshift-ci-robot commented Mar 9, 2021

hardys commented Mar 9, 2021

stbenjam commented Mar 9, 2021

kirankt left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 10, 2021

openshift-bot commented Mar 11, 2021

openshift-ci bot commented Mar 11, 2021 • edited Loading

openshift-bot commented Mar 11, 2021

openshift-ci-robot commented Mar 11, 2021

hardys commented Mar 11, 2021

openshift-cherrypick-robot commented Mar 11, 2021

blkingrh commented Mar 17, 2021

stbenjam commented Mar 17, 2021

kirankt commented Mar 17, 2021

blkingrh commented Mar 17, 2021

blkingrh commented Mar 23, 2021

kirankt commented Mar 23, 2021

blkingrh commented Apr 6, 2021

anthonyfister commented Apr 13, 2021

kirankt commented Apr 13, 2021

blkingrh commented Apr 28, 2021

kirankt commented Apr 29, 2021

blkingrh commented May 6, 2021

kirankt commented May 6, 2021 • edited Loading

evelinec commented May 11, 2021 • edited Loading

kirankt commented May 11, 2021

blkingrh commented May 14, 2021

evelinec commented May 19, 2021

kirankt commented May 19, 2021

evelinec commented May 19, 2021

openshift-ci bot commented Mar 11, 2021 •

edited

Loading

kirankt commented May 6, 2021 •

edited

Loading

evelinec commented May 11, 2021 •

edited

Loading