On 11/16/23 09:51, Dumitru Ceara wrote:
> On 11/16/23 09:44, Ales Musil wrote:
>> On Thu, Nov 16, 2023 at 9:34 AM Dumitru Ceara <dce...@redhat.com> wrote:
>>
>>> On 11/16/23 00:12, Ilya Maximets wrote:
>>>> On 11/15/23 22:20, Mark Michelson wrote:
>>>>> On 11/15/23 12:02, Ilya Maximets wrote:
>>>>>> On 11/15/23 17:00, Dumitru Ceara wrote:
>>>>>>> All of the above were changed to track the latest available releases.
>>>>>>> Initially that seemed like a good idea but in practice, a new release
>>> would
>>>>>>> potentially (silently) cause CI runs that used to pass on given stable
>>>>>>> versions to unexpectedly start failing.
>>>>>>>
>>>>>>> To address that this series pins all versions we can control allowing
>>> us
>>>>>>> to use different values for different branches.
>>>>>>>
>>>>>>> NOTE: the last commit of this series will look slightly different on
>>>>>>> stable branches, e.g., on branches <= 23.06 we need Python <= 3.11 and
>>>>>>> Fedora 37.
>>>>>>>
>>>>>>> The first commit of the series bumps the OVS submodule to include the
>>>>>>> latest fixes to errors reported by recent versions of flake8.
>>>>>>>
>>>>>>> Changes in v2:
>>>>>>> - added first patch to bump OVS submodule
>>>>>>> - addressed Ales' review comments:
>>>>>>> - moved the logic to determine which image to use out of ci.sh;
>>>>>>> it's now in the workflow itself.
>>>>>>> - moved setting of OVN_IMAGE in the same block with all related
>>>>>>> env vars
>>>>>>> - added a note for maintainers in the release process documentation
>>>>>>> to mention that the new workflow will likely have to be triggered
>>>>>>> manually (at least once) when branching for a new release. GitHub
>>>>>>> doesn't allow periodic jobs on non-default branches.
>>>>>>
>>>>>> IMHO, that sounds horrible to maintain. Can we just build things per
>>> run?
>>>>>
>>>>> What if we set up the "Containers" action to run on branch creation?
>>>>> Would that adequately substitute for the manual run that creates the
>>> image?
>>>>>
>>>>> If we only have to run the action once each time a branch is created, I
>>>>> don't think that qualifies as "horrible" to maintain. This could be
>>>>> lumped into our branch creation scripts. However, if it can be
>>> automated
>>>>> as I suggested above, that would be better.
>>>>
>>>> The job will never be re-run, meaning we'll be using the same container
>>>> for many years. In case we'll need an update for this container, it will
>>>> need to be done manually for each branch, except for main.
>>>>
>>>> Also, in the patch, the Cirrus CI is set to use the container from main,
>>>> so this will need to be adjusted for each branch, will need to become
>>>> part of the release patches (?).
>>>>
>>>> Another issue with current implementation of containers is that it's
>>>> a pain for external contributor to change dependencies, because they'll
>>>> have to modify the CI in multiple places in order to build and use their
>>>> own containers.
>>>> And with these new changes it will become worse. For example, I never
>>> use
>>>> 'main' or 'branch-*' branches in my own forks. And CI depends on the
>>> branch
>>>> name now, and it will fall back to an unmaintained old-style-named image.
>>>> Might not be easy to figure out why all your builds are failing,
>>> especially
>>>> if you're new to the project.
>>>>
>>>
>>> We could try to make it build the container image every time on stable
>>> branches and avoid any manual steps.
>>>
>>>>>
>>>>>> How much value these containers actually have?
>>>
>>> I think you mentioned the benefit lower already:
>>> - we can share the artifact (container image) with Cirrus CI; aarch64
>>> container build takes significantly longer
>>> - we can share the artifact with downstream CIs
>>>
>>>>>
>>>>> As patch 2 mentions, it allows for us to use different distro bases for
>>>>> each version.
>>>>
>>>> This can be done by simply specifying 'container: name' in the job,
>>>> GHA will fetch and use that container.
>>>>
>>>>> But I also think that having the container image cached
>>>>> allows for quicker test runs since we are not having to reconstruct the
>>>>> environment each time we perform test runs.
>>>>
>>>> It takes less than 2 minutes to create a full x86 environment. We spend
>>>> more time re-building OVS and OVN for every variant of tests, even though
>>>> most of them are using the same compiler flags.
>>>>
>>>> Anyways, there is no need to push the container into registry, we can
>>> build
>>>> it and pass to a next job via artifacts. I don't think spending extra
>>> 1-2
>>>> minutes per workflow (not job, workflow) is a significant problem, if it
>>>> allows to just build whatever the current dependencies are and not think
>>>> about what kind of outdated build from which branch is going to be used.
>>>>
>>>> The only downside of this is that we can't share artifact with Cirrus CI
>>>> or some downstream CIs, but that also doesn't seem to be a significant
>>>> issue. 1-2 minutes per workflow is not that much. Building the aarch64
>>>> container takes a lot more time, but switching it from fedora to ubuntu
>>>> takes only 10 minutes to build, so might be reasonable to just rebuild it
>>>> on-demand as well.
>>>>
>>>
>>> Ales, why do we need the Cirrus CI image to be Fedora based?
>>>
>>
>> We don't, but it's way easier for me to navigate. I'm not even sure how/if
>> Ubuntu one works for aarch64. We can switch that one, however I don't see
>> any clear benefit in doing so. Also one thing to note, even if it's 1-2
>
> Well, it takes way less time to build, that might be an argument for it.
>
>> minutes (it's probably slightly more) we are losing the ability to have
>> reproducible builds. It's very easy today to take the image and reproduce
>> the same issue IMO.
>>
>
> That's a good point, reproducible builds are important. Storing the
> image as GitHub CI artifact would still allow us to download it and load
> it locally but it's way easier if it's just stored in the ghcr.io register.
>
>>
>>>
>>>> Not a strong opinion, but the branch name thing might be the most
>>> annoying,
>>>> if I understand correctly.
>>>>
>>>
>>> On the (very) short term however, my problem is that I can't merge any
>>> bug fix in OVN because I can't run any CI at the moment. Mostly that's
>>> because of different versions of OVN (and OVS via submodule) python code
>>> that triggers different flake8 warnings with different
>>> flake8/python/distro versions.
>>>
>>> A quick fix for that for now could be to bump OVS submodule again to
>>> fdbf0bb2aed5 ("flake8: Fix E721 check failures.") (which is what patch
>>> 1/3 is doing) but on _all_ stable branches. That's not ideal because
>>> that commit (and its backports) are not part of any release yet.
>>>
>>> I wanted to avoid doing that but it seems we don't have an easy
>>> alternative so I'm starting to think we should just go for the OVS
>>> submodule bump and try to find a better solution for the container
>>> workflow on the longer term.
>>>
Just in case I wasn't clear, what I meant here is applying patch 1/3 to
main and its corresponding stable branch backports bumping the submodule
to the tips of OVS branch-3.2, branch-3.1 and branch-3.0.
>>> What do you guys think?
>>>
>>
>> I'm afraid that we will constantly chase those ghosts of different
>> dependency problems for various branches. And I'm not sure what would be
>> the proper way to handle that. The unfortunate thing is that we don't
>> control directly OvS so some of those "hacks" might be always necessary to
>> have it working.
>>
>
> Isn't that a different discussion? Partially because OVN uses
> "internal" OVS structures directly.
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
