Skip to content

Bug 1931505: [on-prem] Cleanup keepalived vips before starting service #2511

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 1 commit into from
Apr 15, 2021
Merged

Bug 1931505: [on-prem] Cleanup keepalived vips before starting service #2511

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion templates/common/on-prem/files/keepalived.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,21 @@ contents:
done
}

remove_vip()
{
address=$1
interface=$(ip -o a | grep '\s${address}/' | awk '{print $2}')
cidr=$(ip -o a | grep '\s${address}/' | awk '{print $4}')
if [ -n "$interface" ]; then
ip a del $cidr dev $interface
fi
}

set -ex
# Ensure that we don't have stale VIPs configured
Copy link
Member

@mandre mandre Apr 6, 2021

Choose a reason for hiding this comment

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

If we intend to someday remove the workaround then we should reference the upstream keepalived issue. I personally don't see a problem keeping the removal of the VIP in the startup script forever, so up to you if you want to modify the patch.

Copy link
Member Author

@cybertron cybertron Apr 6, 2021

Choose a reason for hiding this comment

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

In the interest of not throwing away the ci passes I have I think I'll leave it alone for now. I can push a small followup to add a reference to the bz that can be merged whenever. If that doesn't make 4.8 it won't be a big deal.

Copy link
Contributor

@dcbw dcbw Apr 12, 2021

Choose a reason for hiding this comment

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

@cybertron Maybe we should add that now, since it had to be rebased in the last 15 minutes anyway?

Copy link
Member Author

@cybertron cybertron Apr 12, 2021

Choose a reason for hiding this comment

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

Yep, good point. Thanks for the reminder.

# See https://bugzilla.redhat.com/show_bug.cgi?id=1931505
remove_vip "{{ onPremPlatformAPIServerInternalIP . }}"
remove_vip "{{ onPremPlatformIngressIP . }}"
declare -r keepalived_sock="/var/run/keepalived/keepalived.sock"
export -f msg_handler
export -f reload_keepalived
Expand Down Expand Up @@ -168,4 +182,4 @@ contents:
- operator: Exists
priorityClassName: system-node-critical
status: {}
{{ end -}}
{{ end -}}