Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

Optional enable_drain_healthcheck, removed status code 000 check #214

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

iamejboy
Copy link
Contributor

Feature or Bug Description

Removed 000 here

if [[ $status_code -eq 000 || $status_code -eq 200 ]]; then

Update put on spec desc, "---skip-drain flag is required in order to delete a deployment" then add "scaling down"

Motivation

Drain script will be more useful to make sure that cluster is healthy before shutting down the local node. Unlike when 000 is added, we assume its scaling down but its not always the case. It could be a node left network/connectivity issue, crashed node, or bosh monit stopped node.

Related Issue

#208
#209
#209 (comment)

@cfdreddbot
Copy link

Hey iamejboy!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cf-gitbot
Copy link
Collaborator

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/160049395

The labels on this github issue will be updated when the story is started.

@@ -26,7 +26,7 @@ for NODE in "${CLUSTER_NODES[@]}"; do
set +e
status_code=$(curl -s -o "/dev/null" -w "%{http_code}" "$NODE:$GALERA_HEALTHCHECK_PORT")
set -e
if [[ $status_code -eq 000 || $status_code -eq 200 ]]; then
if [ $status_code -eq 200 ]; then
Copy link

Choose a reason for hiding this comment

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

Suggested change
if [ $status_code -eq 200 ]; then
if [[ $status_code -eq 200 ]]; then

Double square brackets are to be kept, in order to ensure that we can safely skip quoting around the $status_code variable.

@iamejboy
Copy link
Contributor Author

Hi @APShirley , any chance that this could be merge soon?

Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging this pull request may close these issues.

4 participants