On 11/16/23 13:05, Ilya Maximets wrote: > On 11/16/23 10:17, Dumitru Ceara wrote: >> On 11/16/23 10:02, Dumitru Ceara wrote: >>> 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. > > But the branch name will still be a problem. Every time people use any > branch name other than 'main' for their own development, they'll get an > outdated container, unless they change the CI scripts. Or am I missing > something? >
Yeah, we'd have to make it build the container image every time on non-default branches. That's a problem because the fedora container takes so long. So we'd have to separate the two types of container builds. Seems like a lot of trouble to go through indeed. >>>>>> >>>>>>>> >>>>>>>>> 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. > > FWIW, Fedora takes an hour to build, Ubuntu only takes ~10 minutes. > >>>> >>>>> 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. > > How many times did you use this for debugging in the past year? > If it's not a day-to-day churn, it might be worth to add an extra > step of downloading an artifact. > I'm not saying it's hard to use the artifact directly, just that it's more convenient to get the image from the registry. I don't use it that much, but when I do need it it's way easier for me to do a regular "podman/docker run" pointing it to the registry than to figure out how to download and load the image from the artifact. Anyhow, it's a minor annoyance and can be automated. >>>> >>>>> >>>>>> >>>>>>> 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. > > Bumping submodule to the tips of OVS stable branches seems OK to me. > OK. >>> >> >> .. and we need to pin Python version to 3.11 on branches <=23.03. > > In general, patches 1 and 3 of this set are fine (I didn't test). OK, I'll prepare v3 and backports and post them soon. > The main problem, IMO, is the branch name dependency that makes > day-to-day development harder. > I think it's still useful to be able to pin container versions on stable branches and make them re-usable for other types of (downstream) CI. OTOH, if we have a dockerfile we use for building these CI container images, maybe other CI systems (e.g., the RedHat downstream CI) could just use the dockerfile to periodically pre-build their own environment. Ales, what do you think? >> >>>>>> 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