PE-19888 Add workaround for 2016.1.1 console status check error #63
Conversation
|
Can one of the admins verify this patch? |
|
Refer to this link for build results (access rights to CI server needed): |
|
👍 from me, for what it counts. If we could get this in by this weekend's LTS integration run it would be greatly appreciated. |
lib/beaker-pe/install/pe_utils.rb
Outdated
| if (host['pe_ver'] == '2016.1.1') | ||
| output = on(host, "curl -s -k https://localhost:4433/status/v1/services?level=critical --cert /etc/puppetlabs/puppet/ssl/certs/#{host}.pem --key /etc/puppetlabs/puppet/ssl/private_keys/#{host}.pem --cacert /etc/puppetlabs/puppet/ssl/certs/ca.pem", :accept_all_exit_codes => true) | ||
| else | ||
| output = on(host, "curl -s -k https://localhost:4433/status/v1/services --cert /etc/puppetlabs/puppet/ssl/certs/#{host}.pem --key /etc/puppetlabs/puppet/ssl/private_keys/#{host}.pem --cacert /etc/puppetlabs/puppet/ssl/certs/ca.pem", :accept_all_exit_codes => true) |
There was a problem hiding this comment.
@shaigy rather than repeating this entire command wholesale, I think it would be nicer if we pointed out just the difference for 2016.1.1 for readability. So this would look something like this:
query_params = host['pe_ver'] == '2016.1.1' ? '?level=critical' : ''
output = on(host, "curl -s .... v1/services#{query_params} --cert ...")(that's assuming that query param is the only change, I just glanced at it)
shaigy
force-pushed
the
PE-19888-add-workaround-for-console-status-check-error-in-2016.1.1
branch
from
e40944e to
cb72b95
Compare
|
@kevpl I incorporated the code change. That definitely looks better! |
|
Refer to this link for build results (access rights to CI server needed): |
shaigy
force-pushed
the
PE-19888-add-workaround-for-console-status-check-error-in-2016.1.1
branch
from
cb72b95 to
ed002c9
Compare
|
Refer to this link for build results (access rights to CI server needed): |
shaigy
force-pushed
the
PE-19888-add-workaround-for-console-status-check-error-in-2016.1.1
branch
from
ed002c9 to
11fd19c
Compare
|
@kevpl Can you please review the changes? |
|
Refer to this link for build results (access rights to CI server needed): |
shaigy
force-pushed
the
PE-19888-add-workaround-for-console-status-check-error-in-2016.1.1
branch
from
11fd19c to
0357a14
Compare
|
I'm 👍 on this. Once it comes through testing, I'm down to merge it. @cthorn42 any issues with releasing it as soon as it's finished? Any synchronizations that need to happen, or are we green to cut a new gem? |
There was a problem hiding this comment.
@shaigy Can you please update your commit message to explain what the issue is and why setting the level to critical works around it.
Also, a comment in the code saying this is being done to work around a known issue, with a link to said known issue would be helpful for any future refactorings / code additions.
|
@kevpl I have no issue with releasing this gem. Once Eric's change request gets in I'm all for cutting the new gem. It will let us start to triage the LTS weekend job quicker. |
|
Refer to this link for build results (access rights to CI server needed): |
shaigy
force-pushed
the
PE-19888-add-workaround-for-console-status-check-error-in-2016.1.1
branch
from
0357a14 to
50d28cc
Compare
|
@ericwilliamson I have modified the commit message, can you review? |
|
Refer to this link for build results (access rights to CI server needed): |
👍 on the message - can you please add a comment to the actual code now as well please, then 👍 |
During installation, a classifier-service check is done in beaker-pe to make sure console service is up and running. The classifier status service at the default detail level is broken in 2016.1.1 (PE-14857) and returns an error and state "unknown". The workaround is to query the classifier-service at 'critical' level and check for the console service status.
shaigy
force-pushed
the
PE-19888-add-workaround-for-console-status-check-error-in-2016.1.1
branch
from
50d28cc to
6cd41e4
Compare
|
@ericwilliamson Done! |
|
👍 @kevpl Any other changes you want done? |
|
looks good. Once this passes testing, I'll merge & cut a new beaker-pe for it. |
|
Refer to this link for build results (access rights to CI server needed): |
During installation, a classifier-service check is done in beaker-pe
to make sure console service is up and running. The classifier status
service at the default detail level is broken in 2016.1.1 (PE-14857)
and returns an error and state "unknown". The workaround is to query
the classifier-service at 'critical' level and check for the console
service status.
Please delete any headings that don't apply to this Pull Request (PR).
What's this PR do?
Who would you like to review this PR?
@puppetlabs/integration, @puppetlabs/beaker (repo owners)
Should any of this be tested outside the normal PR CI cycle?
Any background context you want to provide?
Questions for reviewers?