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

Fixes #27122 - Sat 6.6.z scenario #270

Merged
merged 1 commit into from Jul 1, 2019

Conversation

upadhyeammit
Copy link
Contributor

This adds Satellite 6.6.z upgrade scenario.

@theforeman-bot
Copy link
Member

Issues: #27122

Copy link
Member

@kgaikwad kgaikwad left a comment

Choose a reason for hiding this comment

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

Thank you @upadhyeammit.
Overall looks good to me except one inline comment.

end

def compose
add_step(Procedures::Service::Start.new)
Copy link
Member

Choose a reason for hiding this comment

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

There is difference in steps of post-migrations phase from scenario 6.6 to 6.6.z.
Below steps are missing which might be needed for upgrading 6.5 to 6.6.z.

add_step(Procedures::ForemanDocker::RemoveForemanDocker.new)
add_step(Procedures::Foreman::ApipieCache.new)

@xprazak2, please confirm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, if someone goes from 6.5 directly to 6.6.z, docker should be dropped as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kgaikwad @xprazak2 As maintain wont have upgrade path available from 6.5 to 6.6.z I think we can skip these two steps ?

Copy link
Member

@kgaikwad kgaikwad Jun 28, 2019

Choose a reason for hiding this comment

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

+1. in that case, we can skip those steps.

Copy link
Contributor

Choose a reason for hiding this comment

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

@upadhyeammit @kgaikwad Sorry for replying late here. But just a scenario came in mind. Let's consider what if user is performing upgrade from 6.5 to 6.6 and after yum update, somehow installer failed in first try and user wants to retry upgrade. At this point we will have 6.6.z target version and considering installer completed successfully in second run, but, this will not remove docker as we have not defined this steps for 6.6.z scenario. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

The current behavior should remember that the upgrade from 6.5 to 6.6 is in progress, until it finishes successfully, so in this case, it would not have 6.6.z even though there are already packages from 6.6 installed.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, true. It may affect only when somehow data.yml is removed/changed.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @ntkathole. To avoid this case, merging PR-272 which will add docker removal steps to 6.6.z scenario.

If user tries to do upgrade for 6.6.z after successful 6.6 upgrade, nothing to worry about it as Docker removal procedure has condition which verifies presence of docker_package before proceeding further.

@kgaikwad
Copy link
Member

@iNecas, @jameerpathan111,
do you have any comments? or may I merge this?

@jameerpathan111
Copy link
Contributor

@iNecas, @jameerpathan111,
do you have any comments? or may I merge this?

@kgaikwad It is fine with me.

@iNecas
Copy link
Member

iNecas commented Jun 28, 2019

@iNecas no more comments from me

@kgaikwad kgaikwad merged commit 30eaa02 into theforeman:master Jul 1, 2019
@kgaikwad
Copy link
Member

kgaikwad commented Jul 1, 2019

Thank you @upadhyeammit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants