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

Document workaround for the broken snactor #799

Merged
merged 2 commits into from
Oct 7, 2022
Merged

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Oct 6, 2022

prefer having this documented instead answering everyone questions why it does not work when it is in documentation. At least until it is fixed.

@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp to Packit allowed forge projectsin the Copr project settings.

@github-actions
Copy link

github-actions bot commented Oct 6, 2022

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

  • review please to notify leapp developers of review request
  • /packit copr-build to submit a public copr build using packit

To launch regression testing public members of oamg organization can leave the following comment:

  • /rerun to schedule basic regression tests using this pr build and leapp-repository*master* as artifacts
  • /rerun 42 to schedule basic regression tests using this pr build and leapp-repository*PR42* as artifacts
  • /rerun-all to schedule all tests (including sst) using this pr build and leapp-repository*master* as artifacts
  • /rerun-all 42 to schedule all tests (including sst) using this pr build and leapp-repository*PR42* as artifacts

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

@@ -328,6 +328,17 @@ data is stored in the database for the current session:
snactor run --save-output IPUWorkflowConfig
```

Because of a bug in leapp, this does not work and gives the following error:
Copy link
Member

@fernflower fernflower Oct 6, 2022

Choose a reason for hiding this comment

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

Technically it's not a bug or something that can be fixed for good, as snactor is a development tool and it is not being run on the machine-to-be-upgraded, so it has no knowledge of what system leapp is planned to be run for and thus needs this information to be passed this way or another. Setting LEAPP_UPGRADE_PATH_TARGET_RELEASE and LEAPP_UPGRADE_PATH_FLAVOR to the values you mentioned is the way to say "Run leapp actors as you would for a 7to8 upgrade on a typical rhel system".

Could you please rephrase the description a bit? Maybe something like After leapp started supporting several upgrade paths snactor requires target system and upgrade flavor specification for a correct execution. To get around the possible leapp ModelViolationError ... please set the following environment variables prior to running snactor.

And thanks a lot for the patch, that's definitely an awesome addition to the dev docs!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I reworded the changed based on your suggestion.

Giving this another though, would it make sense to have this information somewhere inside of the snactor, when it is executed without these variables, reporting that it might not work for all the workflow related tasks, at least as a warning? Or even make it mandatory for execution of snactor? Or provide some user-friendly commandline switches for snactor to set this for you?

Copy link
Member

@fernflower fernflower Oct 7, 2022

Choose a reason for hiding this comment

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

You are definitely right, we should add this information to snactor. I'll create an issue for that.

prefer having this documented instead answering everyone questions why it does not work when it is in documentation. At least until it is fixed.
@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp to Packit allowed forge projectsin the Copr project settings.

@leapp-bot
Copy link
Collaborator

This PR has been linked in issue tracker (#OAMG-7725).

Co-authored-by: Petr Stodůlka <xstodu05@gmail.com>
@packit-as-a-service
Copy link

Your git-forge project github.com/oamg/leapp has permissions to build in @oamg/leapp Copr project configured in Packit. However, we migrated to the solution where you can configure the allowed git-forge projects in Copr yourself and will remove the configuration in Packit for the allowed projects soon. Therefore, please, add this git-forge project github.com/oamg/leapp to Packit allowed forge projectsin the Copr project settings.

Copy link
Member

@fernflower fernflower left a comment

Choose a reason for hiding this comment

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

Looks perfect now, thanks!

Copy link
Member

@pirat89 pirat89 left a comment

Choose a reason for hiding this comment

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

Same as Ina. Thanks!

@pirat89 pirat89 merged commit d8e01d7 into oamg:master Oct 7, 2022
@pirat89 pirat89 added docs changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant labels Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-checked The merger/reviewer checked the changelog draft document and updated it when relevant docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants