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

(PE-17825) Update do_higgs_install method with new installation log #34

Merged
merged 2 commits into from
Oct 11, 2016

Conversation

phongdly
Copy link
Contributor

@phongdly phongdly commented Oct 6, 2016

Prior to this PR, the PE the do_higgs_install method waits for the below PE installation log:
"Please go to https://higgs_installer_web_server:3000 in your browser to continue installation"

However, newer PE, for example in Davis builds, that line of log has changed to be:
"#Go to https://higgs_installer_web_server:3000 in your browser to continue installation"

This PR is a simple fix for this by only searching for the substring:
"o to https://higgs_installer_web_server:3000 in your browser to continue installation"

Prior to this PR, the PE the do_higgs_install method waits for the below PE installation log:
"Please go to https://higgs_installer_web_server:3000 in your browser to continue installation"

However, newer PE, for example in Davis builds, that line of log has changed to be:
"#Go to  https://higgs_installer_web_server:3000 in your browser to continue installation"

This PR is a simple fix for this by only searching for the substring:
"o to  https://higgs_installer_web_server:3000 in your browser to continue installation"
@puppetlabs-jenkins
Copy link
Contributor

Can one of the admins verify this patch?

Copy link
Contributor

@highb highb left a comment

Choose a reason for hiding this comment

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

Ah, the string overlaps. 👍 then

@@ -899,7 +899,7 @@ def do_higgs_install host, opts

#wait for output to host['higgs_file']
#we're all done when we find this line in the PE installation log
higgs_re = /Please\s+go\s+to\s+https:\/\/.*\s+in\s+your\s+browser\s+to\s+continue\s+installation/m
higgs_re = /o\s+to\s+https:\/\/.*\s+in\s+your\s+browser\s+to\s+continue\s+installation/m
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yes this should fix the problem. But doesn't it break compatibility with older versions? Perhaps you should have it use the version_is_less to determine which string to grep for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point, yes, it will break the older versions because we just dropped the word "Please" in Davis build. I will update the PR and repush it quickly

@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-beaker.delivery.puppetlabs.net//job/qe_beaker-pe_btc-intn/117/

@puppetlabs-jenkins
Copy link
Contributor

Refer to this link for build results (access rights to CI server needed):
http://jenkins-beaker.delivery.puppetlabs.net//job/qe_beaker-pe_btc-intn/120/

Copy link
Contributor

@highb highb left a comment

Choose a reason for hiding this comment

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

👍 from me

@highb
Copy link
Contributor

highb commented Oct 11, 2016

@kevpl If you are available to take a look at this. Otherwise, I'll probably go ahead a merge this sometime today.

@kevpl kevpl merged commit 52921be into puppetlabs:master Oct 11, 2016
@kevpl kevpl mentioned this pull request Dec 5, 2016
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.

4 participants