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

upgrade: Report the admin server upgrade error if knife has failed #2306

Merged
merged 1 commit into from
Feb 15, 2017
Merged

upgrade: Report the admin server upgrade error if knife has failed #2306

merged 1 commit into from
Feb 15, 2017

Conversation

jsuchome
Copy link
Member

No description provided.

@jsuchome
Copy link
Member Author

echo "12.2" >> $UPGRADEDIR/admin-server-upgraded-ok
ret=$?
if [ $ret != 0 ]; then
report_failure($ret, "Setting the platform to suse-12.2 has failed.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it makes a bit difference here. But shouldn't we acutally exit here, instead of just skipping the admin-server-upgraded-ok step

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was thinking about that, and I'm not sure. It may actually make sense to run the rest of the script anyway, and user could try to just fix these os values on the upgraded server.

I'm not sure if failing on Cloud6 side makes sense, as the crowbar has been already upgraded - I think restarting the whole step will not be possible. (This is related also to the other possible failure actually)

Copy link
Member Author

Choose a reason for hiding this comment

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

That said ... if users do not restart the step using the progress library, it stays as failed and they cannot continue. Sigh.

Copy link
Contributor

@AbelNavarro AbelNavarro left a comment

Choose a reason for hiding this comment

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

I like the refactor but agree with @rhafer that exiting on that check may be is the best option.

@dirkmueller dirkmueller added this to the Cloud 7 Update1 milestone Feb 14, 2017
@jsuchome
Copy link
Member Author

agree with @rhafer that exiting on that check may be is the best option.

This is really complicated. I think that once packages are upgraded, we need to let the admin server reboot and handle possible failures on Cloud7 side. Otherwise I'm not sure what the UI should tell the user if it detects the failure during this step: upgraded crowbar has already Cloud7 sources...

@rhafer
Copy link
Contributor

rhafer commented Feb 15, 2017

This is really complicated. I think that once packages are upgraded, we need to let the admin server reboot and handle possible failures on Cloud7 side.

In that case wouldn't it make sense to just run the knife command before upgrading the packages? (i.e. after stopping chef-client)

@jsuchome
Copy link
Member Author

In that case wouldn't it make sense to just run the knife command before upgrading the packages? (i.e.
after stopping chef-client)

That might work actually... I have to test it

@jsuchome jsuchome merged commit fc586bc into crowbar:stable/3.0 Feb 15, 2017
@jsuchome jsuchome deleted the check-os-change branch February 15, 2017 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants