Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove onPremPlatformKeepalivedEnableUnicast function #3174

Merged

Conversation

EmilienM
Copy link
Member

This isn't used anymore since we enabled Unicast for all on-prem
platforms which use Keepalived, via #3016.

This isn't used anymore since we enabled Unicast for all on-prem
platforms which use Keepalived, via openshift#3016.
@EmilienM
Copy link
Member Author

/test e2e-openstack

@EmilienM
Copy link
Member Author

/test e2e-metal-ipi
/test e2e-vsphere-upgrade

@EmilienM
Copy link
Member Author

/cc mandre

@openshift-ci openshift-ci bot requested a review from mandre May 31, 2022 19:31
Copy link
Member

@mandre mandre left a comment

Choose a reason for hiding this comment

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

/lgtm
/assign @cybertron

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 1, 2022
@cybertron
Copy link
Member

I actually left these on purpose because I'm not sure what the order of operations for upgrades is and we've had problems in the past with old templates being rendered with new code (or vice versa). If we wait to remove these functions until 4.12 then we don't have to worry about that.

I guess if someone can verify that this doesn't cause any problems on upgrade we can go ahead and merge it, but otherwise I'd prefer to just wait.

/hold
until either 4.12 or verification of upgrades

@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 Jun 1, 2022
@mandre
Copy link
Member

mandre commented Jun 2, 2022

I actually left these on purpose because I'm not sure what the order of operations for upgrades is and we've had problems in the past with old templates being rendered with new code (or vice versa). If we wait to remove these functions until 4.12 then we don't have to worry about that.

That's good to know, I hadn't thought about the possibility of having an old template using new binaries for rendering, although I don't exactly understand how this would happen 😄
We're past feature freeze anyway, and this would need a BZ if we wanted to merge the patch now. It clearly doesn't qualify as a bug, so it makes total sense to wait until master re-opens, for both safety and convenience.

@cybertron
Copy link
Member

/test e2-vsphere-upgrade
/test e2e-metal-ipi
/test e2e-openstack
/hold cancel
/lgtm

4.12 is open, so this is good to go. Just re-running the on-prem jobs to make sure nothing broke in the meantime.

@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 Jun 29, 2022
@cybertron
Copy link
Member

/test e2e-metal-ipi
/test e2e-openstack
/test e2e-vsphere-upgrade

@sinnykumari
Copy link
Contributor

/retest

1 similar comment
@cybertron
Copy link
Member

/retest

@mandre
Copy link
Member

mandre commented Aug 16, 2022

This should be good to go now, I believe.
/assign sinnykumari

@sinnykumari
Copy link
Contributor

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 19, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cybertron, EmilienM, mandre, sinnykumari

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

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

openshift-ci bot commented Aug 19, 2022

@EmilienM: all tests passed!

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.

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.

5 participants