Packit-service: Downstream release proposal does not respect requested dist-git branches

[bocekm]

The packit conf for convert2rhel says that the dist-git branches to be used for downstream release are el6 and epel7:

- job: propose_downstream
  trigger: release
  metadata:
    dist_git_branches:
    - el6
    - epel7

Yet, after creating a new upstream release, packit has created a downstream PR only for the master dist-git branch (which is used for Fedora Rawhide): https://src.fedoraproject.org/rpms/convert2rhel/pull-request/2.

[csomh]

We'll have to test this on our side to see what's wrong. I was not able to find the relevant logs for this event.

[lachmanfrantisek]

Packit part (config)

The expansion is done in the packit/config/aliases.py:
https://github.com/packit/packit/blob/c571c810bf633477cd470bb2652a4461cab8c1c0/packit/config/aliases.py#L116-L124

def get_branches(*name: str, default=DEFAULT_VERSION) -> Set[str]:
    """
    Expand the aliases to the name(s) and transfer to the dist-git branch name.
    :param name: name(s) of the system and version (e.g. "fedora-stable"/"fedora-30")
            or branch name (e.g. "f30"/"epel8")
    :param default: used if no positional argument was given
    :return: set of dist-git branch names
    """

Aliases are defined here:
https://github.com/packit/packit/blob/c571c810bf633477cd470bb2652a4461cab8c1c0/packit/config/aliases.py#L28-L33

Tests are here (note that we have both el6 and epel7 there):
https://github.com/packit/packit/blob/c571c810bf633477cd470bb2652a4461cab8c1c0/tests/unit/test_config_aliases.py#L133

JobMetadataConfig:
https://github.com/packit/packit/blob/c571c810bf633477cd470bb2652a4461cab8c1c0/packit/config/job_config.py#L60

JobMetadataSchema:
https://github.com/packit/packit/blob/c571c810bf633477cd470bb2652a4461cab8c1c0/packit/schema.py#L221

Note that we have a deprecation there:

class JobMetadataSchema(MM23Schema):
    """ Jobs metadata. """

    targets = fields.List(fields.String())
    timeout = fields.Integer()
    owner = fields.String()
    project = fields.String()
    dist_git_branches = fields.List(fields.String())
    branch = fields.String()
    scratch = fields.Boolean()
    list_on_homepage = fields.Boolean()
    preserve_project = fields.Boolean()
    additional_packages = fields.List(fields.String())
    additional_repos = fields.List(fields.String())

    @pre_load
    def ordered_preprocess(self, data, **_):
        for key in ("dist-git-branch", "dist_git_branch"):
            if key in data:
                logger.warning(
                    f"Job metadata key {key!r} has been renamed to 'dist_git_branches', "
                    f"please update your configuration file."
                )
                data["dist_git_branches"] = data.pop(key)
        for key in ("targets", "dist_git_branches"):
            if isinstance(data.get(key), str):
                # allow key value being specified as string, convert to list
                data[key] = [data.pop(key)]
        return data

Tests are here:
https://github.com/packit/packit/blob/c571c810bf633477cd470bb2652a4461cab8c1c0/tests/unit/test_config.py#L130-L171

Service part (handler)

https://github.com/packit/packit-service/blob/2aac03d108e24913584c89360e271964a311e088/packit_service/worker/handlers/github_handlers.py#L147-L228

Specifically this:

https://github.com/packit/packit-service/blob/2aac03d108e24913584c89360e271964a311e088/packit_service/worker/handlers/github_handlers.py#L180-L182

[stale[bot]]

This issue has been marked as stale because it hasn't seen any
activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed
by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are
never marked as stale.)

[bocekm]

Please do not close it. We still plan to fix it under https://issues.redhat.com/browse/OAMG-3828.

[stale[bot]]

This issue has been marked as stale because it hasn't seen any
activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed
by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are
never marked as stale.)

[bocekm]

Just for a public record, we're moving convert2rhel off EPEL due to EPEL 6 being dead. For that reason we won't need the propose_downstream feature anymore so we won't be fixing it under https://issues.redhat.com/browse/OAMG-3828.
Now it's up to the Packit team to decide if they wish to have this feature working. @lachmanfrantisek, removed the stale label, so it still seems relevant :)

[lachmanfrantisek]

@bocekm Yes, you are right. (And thanks for the info.) But we want to make sure it will not affect anyone else and @mmuzila is willing to take a look.

[lachmanfrantisek]

@mmuzila See my comment for more info or let me know if you need some help.

[mmuzila]

It seems that this issue has been solved already.

There is also a test case that covers this issue partially (test_dist_git_push_release_handle_multiple_branches() at tests/integration/test_release_event.py).

I tried to reproduce this issue also in environment more resembling real deployment - just in case something strange happens during download of .packit.yaml from GitHub and its parsing. I created a fork of convert2rhel repo (mmuzila/convert2rhel), set webhooks and created a test release. The content (json) of HTTP request that was fired by GitHub was recorded. A test was created that sent this json to packit-service (running in container). Everything worked right and packit-service attempted to create a PR at src.fedoraproject.org for every dist-git-branch specified in .packit.yaml (these attempts failed, as expected, because of missing authentication information in configuration of packit-service).

@lachmanfrantisek, what do you think about it?

[lachmanfrantisek]

@mmuzila Thanks for the investigation!

Looks like we can close the issue. We have a test-case and you are right that there a lot of changes since it was created.