Add RHUI configuration #1142
Add RHUI configuration #1142
Conversation
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 mergeable.
Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build. If you need a different version of leapp from PR#42, use It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported,
[Deprecated] To launch on-demand regression testing public members of oamg organization can leave the following comment:
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 contact leapp-infra. |
| class Transaction_ToKeep(Config): | ||
| section = "transaction" | ||
| name = "to_keep" | ||
| type_ = fields.List(fields.String(), default=[ | ||
| "leapp", | ||
| "python2-leapp", | ||
| "python3-leapp", | ||
| "leapp-repository", | ||
| "snactor", | ||
| ]) | ||
| description = """ | ||
| List of packages to be kept in the upgrade transaction. | ||
| """ |
There was a problem hiding this comment.
Just trying to think why it cannot be same as with models? I mean:
| class Transaction_ToKeep(Config): | |
| section = "transaction" | |
| name = "to_keep" | |
| type_ = fields.List(fields.String(), default=[ | |
| "leapp", | |
| "python2-leapp", | |
| "python3-leapp", | |
| "leapp-repository", | |
| "snactor", | |
| ]) | |
| description = """ | |
| List of packages to be kept in the upgrade transaction. | |
| """ | |
| class Transaction_ToKeep(Config): | |
| section = "transaction" | |
| to_keep = fields.List(fields.String(), default=[ | |
| "leapp", | |
| "python2-leapp", | |
| "python3-leapp", | |
| "leapp-repository", | |
| "snactor", | |
| ]) | |
| """ | |
| List of packages to be kept in the upgrade transaction. | |
| """ |
There was a problem hiding this comment.
Since we've settled on yaml as the actual file system format, I think that loading more information into fields will be fine (default and type_). to_keep and the docstring treatment were things I didn't know about. I think they'll be fine too.
One ramification of relying on fields (I think in any incarnation) is that we should allow mapping values. I heard that those were specifically excluded from the present fields because they were considered too complex and unnecessary for Models. I do not think those are true for config values. Yaml has dictionaries as basic data types and in cases where you want to group record-style information, yaml mappings make the most sense.
hosts:
balancer.example.com: 192.168.100.2
git.example.com: 192.168.100.3I don't know if we'll want to keep field.Mapping (or field.Dict) from being used in models or not care now that we have a need for it for a different purpose.
There was a problem hiding this comment.
@abadger I was thinking about that during weekend, as I think I misunderstood you originally after reading this your comment and the example. I thought that this problem is addressed by fields.JSON which we can allow for configs to not be so strict. However, thinking about that, I think you provided totally valid point about having something like a Map/Mapping/Dict field, which could have variable keys (like hosts, where we do not know number of such items, nor specific keys itself). But we could on the other hand still define what will be values like in case of other fields. So the use for the example above, I could imagine to allow something like:
class MyConfig(Config):
hosts = fields.Mapping(fields.String) # or e.g. fields.Mapping(value_type=fields.String)
Where we can define what will be values, hence the key is expected to be always string (no sure whether we would need anything else for keys than string). For the completness, I can imagine we could resolve it also differently, but usually it would lead to some magical tweaks around serialisation/deserialisation, or little bit worse UX, e.g. to require user to write config like:
hosts:
- hostname: balancer.example.com
- ip: 192.168.100.3
Which is doable and still kind of reasonable, but thinking about the trade-off, it make sense to me to be less strict here. The benefix of the "Mapping" field is, that we can still define properly structure of expected values, so the only unknown it's value/number of the key(s), which in this case it's not so problematic. In case of the JSON field, it's completely unknown (nor key, nor value) and reduce our possibilities of the validation much more.
@abadger Do I understand correctly your idea?
Just to provide context why we usually do not allow fields.JSON in Models inside our official repositories. It's because the JSON field cannot be verified for the content. From the point, we can verify the structure is JSON, but we cannot verify it contains particular fields / content inside, so the structure can vary and we couldn't guarantee that part of this structure will be always a specific key/item (due to bugs, or changes in actor that produce a particular msg, so some other actors - e.g. custom ones - could start to be broken without clean visibility of the problem). Having strict structure in models, provides clear definition of the content and people can easily see it.
abadger
force-pushed
the
actor-config
branch
2 times, most recently
from
bee18dd to
08fb1e6
Compare
abadger
force-pushed
the
actor-config
branch
from
08fb1e6 to
fd99333
Compare
|
/packit test oamg/leapp#870 |
3 similar comments
|
/packit test oamg/leapp#870 |
|
/packit test oamg/leapp#870 |
|
/packit test oamg/leapp#870 |
abadger
force-pushed
the
actor-config
branch
7 times, most recently
from
4e4ad3a to
40260c0
Compare
abadger
force-pushed
the
actor-config
branch
12 times, most recently
from
4af8f20 to
98630ff
Compare
|
/packit copr-build |
|
I was able to upgrade RHEL 8 to 9 x86 successfully using a personal RHUI and custom source and target client RPM names. Well done team! I did try RHEL 7to8 as well, but ran into the glob error mentioned in the description of leapp PR 870. |
MichalHe
force-pushed
the
actor-config
branch
from
ea722ce to
3508187
Compare
MichalHe
force-pushed
the
actor-config
branch
from
9d60346 to
8c378f6
Compare
MichalHe
changed the title
|
added a squash commit modifying %files section covering correct mark of configuration files in the actor_conf.d directory (if any are present in future, provided by the rpm) and making the directory part of the rpm, to fix: |
There was a problem hiding this comment.
@MichalHe lgtm, just the schema should be updated to be more consistent regarding defaults (see comments). Also I will try to do the upgrade with updated config yet as we discussed.
also, do not forget to drop the config file from the rpm and polish commits
(including better explanation in the commit updating target user spaces creator actor, speaking about the check of removal files based on whether they are owned by rpm or not)
MichalHe
force-pushed
the
actor-config
branch
from
99d1afc to
43a054c
Compare
|
tested manually for IPU 7 -> 8 and 8 -> 9 on RHUI AWS. Note, config for IPU 7 -> 8 on AWS looks like this: rhui:
use_config : True
source_clients: [ rh-amazon-rhui-client ]
target_clients: [ rh-amazon-rhui-client ]
cloud_provider: aws
upgrade_files:
/usr/share/leapp-repository/repositories/system_upgrade/common/files/rhui/aws/amazon-id.py: /usr/lib/python2.7/site-packages/dnf-plugins/
/usr/share/leapp-repository/repositories/system_upgrade/common/files/rhui/aws/leapp-aws.repo : /etc/yum.repos.d/
/usr/share/leapp-repository/repositories/system_upgrade/common/files/rhui/aws/cdn.redhat.com-chain.crt : /etc/pki/rhui/product/
/usr/share/leapp-repository/repositories/system_upgrade/common/files/rhui/aws/content-rhel8.crt : /etc/pki/rhui/product/
/usr/share/leapp-repository/repositories/system_upgrade/common/files/rhui/aws/content-rhel8.key : /etc/pki/rhui
/usr/share/leapp-repository/repositories/system_upgrade/common/files/rhui/aws/rhui-client-config-server-8.crt : /etc/pki/rhui/product/
/usr/share/leapp-repository/repositories/system_upgrade/common/files/rhui/aws/rhui-client-config-server-8.key : /etc/pki/rhui
rhui_target_repositories_to_use:
- rhel-8-appstream-rhui-rpms
- rhel-8-baseos-rhui-rpms |
The leapp actors configuration feature is present since leapp-framework 6.0. Update the dependencies to ensure the correct version of the framework is installed on the system. Also, leapp requirements have been updated - requiring python3-PyYAML as it requires YAML parser, bumping leapp-framework-dependencies to 6. Address the change in leapp-deps metapackage to satisfy leapp dependencies during the upgrade process.
pirat89
force-pushed
the
actor-config
branch
2 times, most recently
from
0b7058f to
05f15b2
Compare
|
@MichalHe I've drop the tmp commits. Also I made one small change. I'Ve added etc/leapp/actor_confid.d dir to the repo (with Also added a commit that ensures that all .gitkeep files are deleted when building rpm. so they are not installed then. |
Add additional build steps to the specfile that create the actor configuration directory. The directory is owned by the package, so it gets removed when the user uninstalls leapp. Also prepared some comment lines for future when we will want to include some configuration files as part of the rpm.
We have several .gitkeep files in the repo as we want to have some directories present in git however these directories are empty otherwise. This is common hack to achieve this, but we do not want to have these files really in the resulting RPMs. So we just remove them.
Load actor configuration when running `leapp upgrade` or `leapp preupgrade`. The configuration is loaded, saved to leapp's DB, and remains available to all actors via framework's global variable.
Introduce a common configuration definition for RHUI related decisions. The configuration has an atomic nature - if the user wants to overwrite leapp's decisions, he/she must overwrite all of them. Essentially, all fields of the RHUI_SETUPS cloud map entry can be configured. Almost no non-empty defaults are provided, as no reasonable defaults can be given. This is due to all setup parameters are different from provider to provider. Therefore, default values are empty values, so that it can later be detected by an actor whether all fields of the RHUI config has been filled. Jira ref: RHEL-56251
Extend the check_rhui actor to read user-provided RHUI configuration. If the provided configuration values say that the user wants to overrwrite leapp's decisions, then the patch checks whether all values are provided. If so, corresponding RHUIInfo message is produced. The only implemented safe-guards are those that prevent the user from accidentaly specifying a non-existing file to be copied into the scrach container during us preparing to download target userspace content. If the user provides only some of the configuration values the upgrade is terminated early with an error, providing quick feedback about misconfiguration. The patch has been designed to allow development of upgrades on previously unknown clouds (clouds without an entry in RHUI_SETUPS). Jira ref: RHEL-56251
Extend the CurrentActorMocked class to accept a `config` value, allowing developers to mock actors that rely on configuration. A library function `_make_default_config` is also introduced, allowing to instantiate default configs from config schemas.
We copy files into the target userspace when setting up target repository content. If this file is named equally as some of the files installed by the target RHUI client installed during early phases of target userspace setup process, we would delete it in cleanup. Therefore, if we copy a repofile named /etc/yum.repos.d/X.repo and the target client also owns a file /etc/yum.repos.d/X.repo, we would remove it, making the container loose access to target content. This patch prevents us from blindly deleting files, keeping files that are owned by some RPM (usually that would be the target RHUI client).
pirat89
force-pushed
the
actor-config
branch
from
05f15b2 to
37e9f77
Compare
pirat89
added
the
changelog-checked
Add configuration options for leapp's RHUI handling and RPM transactions.
RHUI Configuration
Example of RHUI configuration:
Leapp reads the configuration, checking the
use_configfield to know whether the values from config should be used, instead of leapp's decision routines. Ifuse_config=True, leapp will not do any automatic decisions about RHUI whatsoever. Therefore, the configured values determine what leapp will do.The following configuration options are added:
source_clientstarget_clientscloud_providerupgrade_filesupgrade_filesare removed.enabled_target_repositoriesleapp --enablerepoKnown limitations
Using a repofile withing
upgrade_filesthat uses different repoids than the target client(s) in combination withenable_target_repositoriescauses DNF transaction check to fail.Jira refs:
Acceptance criteria for RHUI configuration:
leapp-rhui-aws-hathat does not require)rhui_repositoriesa required field (or rename totarget_repoids_to_enable)cloud_provider = 'some_random_providerhas been tried (withenabled_target_repositories), there were no problems - no code modifications are required.