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

ipfailover - user check script overrides default #12509

Merged
merged 1 commit into from Jan 18, 2017

Conversation

pecameron
Copy link
Contributor

@pecameron pecameron commented Jan 16, 2017

When the user supplies a check script, that check script replaces the
default check script.

See openshift-docs PR 3501

bug bz1410721
https://bugzilla.redhat.com/show_bug.cgi?id=1410721

Signed-off-by: Phil Cameron pcameron@redhat.com

@pecameron
Copy link
Contributor Author

@knobunc PTAL

Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -44,6 +44,8 @@ function generate_global_config() {

#
# Generate VRRP checker script configuration section.
# When port is 0 always pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, upon reflection, I think this is wrong.

If they give us a check script, always use it
If the port is non-zero set up the tcp check
Otherwise, skip the test

When the user supplies a check script, that check script replaces the
default check script.

bug bz1410721
https://bugzilla.redhat.com/show_bug.cgi?id=1410721

Signed-off-by: Phil Cameron <pcameron@redhat.com>
Copy link
Contributor

@knobunc knobunc left a comment

Choose a reason for hiding this comment

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

LGTM

@knobunc
Copy link
Contributor

knobunc commented Jan 17, 2017

@stevekuznetsov PTAL

@pecameron
Copy link
Contributor Author

@knobunc You still have Changes requested set...
Is there anything else before merge?

@knobunc
Copy link
Contributor

knobunc commented Jan 17, 2017

@pecameron I think you need to reload the page. I approved, but asked @stevekuznetsov for a review since he is the bash ninja.

@pecameron
Copy link
Contributor Author

@knobunc Is this ready to merge?

@knobunc
Copy link
Contributor

knobunc commented Jan 18, 2017

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 82b3d97

@openshift-bot
Copy link
Contributor

openshift-bot commented Jan 18, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13000/) (Base Commit: cf4e84e) (Image: devenv-rhel7_5713)

@openshift-bot openshift-bot merged commit 33b5741 into openshift:master Jan 18, 2017
@pecameron pecameron deleted the bz1410721 branch January 19, 2017 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants