February 2021 - Zuul-discuss - lists.opendev.org

install and use ansible collections in zuul v3 ansible native job?
by Marios Andreou 23 Nov '21

23 Nov '21

hi folks long story short: can we install and use ansible collections in a zuul v3/ansible native job? In the tripleo-ci squad we are trying to use the tripleo_overcloud_image_build role from the tripleo-operator-ansible collection [1]. We are already using other roles from this collection - e.g. there [2]. This is possible because in that case we have zuul calling bash calling ansible... which means we get to configure collections in the bash bit setting ANSIBLE_COLLECTIONS_PATHS [3] before invoking ansible. For the case I'm having trouble with - what i refer to as ansible native - here https://review.opendev.org/#/c/733818/33/roles/oooci-build-images/tasks/mai… we have a zuul job playbook directly trying to install and use a collection. Currently it fails because instead of collections: (doesn't work " 'collections' is not a valid attribute? [4] ) I am trying to use include_role but then it can't find the collection's modules "ERROR! no action detected in task" [5]. I tried to set it in ansible.cfg but that won't work and anyway it is surely the wrong approach. The one thing left to try is to explicitly require ansible 2.9 for the job - but wanted to reach out and see if anyone else has any problems trying to install and use ansible collections (with modules importantly) as part of a zuul v3/ansible native job. thanks for reading and for any comments, marios [1] https://github.com/openstack/tripleo-operator-ansible/tree/master/roles/tri… [2] https://opendev.org/openstack/tripleo-quickstart-extras/src/commit/55e74bb8… [3] https://opendev.org/openstack/tripleo-quickstart/src/commit/299131bcc6dca86… [4] https://23fd78c0f6b01b9c4209-59c294b8f46eb75cda2bd004e2a1c97f.ssl.cf1.rackc… [5] https://2063b7e2bb67f86f8ec5-87fe67c942382c6ee875c514bffa4f21.ssl.cf2.rackc…

3 8

Type annotations
by jim＠acmegating.com 17 Mar '21

17 Mar '21

Hi, I'd like to propose that we remove type annotations and checking from our code. I don't think they are providing much value, but they come at a significant cost. They have very rarely (I think perhaps once or twice) caught errors. Meanwhile, we aren't going to relax our test coverage requirements, which should catch most things anyway. On the other hand, they cause the code to be much less readable and I believe that will ultimately lead to more errors. Developer and reviewer fatigue are important things to keep in mind. Some examples where I think annotations are particularly unhelpful: self.on_connect_listeners: List[Callable[[], None]] = [] Or (note the duplication): def __init__(self, ... info: Optional[zuul.model.WebInfo] = None): self.info: Optional[zuul.model.WebInfo] = info We've also had some occurrences where we ended up fighting the annotation system. I've mentioned to several of you that I do very much enjoy the benefits of statically typed languages (I write C code for fun), but I don't think that the type hints in Python are very well executed. Many of us chose to use Python instead of Java specifically for the dynamic typing and as a reaction against the extensive type requirements. The type hinting that is being added to Zuul seems like it carries the costs of Java without the benefits. We started this as a small experiment. We said that we would run the type checker in a mode where it would operate on implicit type information, thereby minimizing the additional cognitive load on authors and reviewers. We then started adding some minimal type information in strategic places to help it out. However, I'm now reviewing changes that are literally 3x larger than they otherwise would be because of all the additional repetitive type information going into them. I'm glad to try experiments like these, but I think this one has grown to the point where we need to decide if we want to go all-in on type annotations or remove them. I feel fairly strongly that the cost to authors and reviewers far outweighs the benefits, and we should make everyone's lives easier by removing them. -Jim

10 18

Last call for 4.0 deprecations
by jim＠acmegating.com 16 Feb '21

16 Feb '21

Hi, We've merged the SQL database requirement, and as I write this, the ZooKeeper connection requirement patches are working their way through the gate. Those are the last big items blocking the 4.0 release. Before we issue the release, I'd like to ask folks to take a look for anything we've said that we would like to deprecate or remove in a major version bump to make sure we don't miss the opportunity. At this point, I don't think we should entertain last-minute new ideas, but rather, if we put something in the documentation quite some time ago saying that it would be removed in the future, let's make sure we take the opportunity to consider that now. Also, if anyone can think of any other blockers for 4.0, speak now please. :) Otherwise, I think we can cut the release very soon. -Jim

1 1

Scale out scheduler
by jim＠acmegating.com 14 Feb '21

14 Feb '21

Hi, The main blocker for releasing 4.0 at this point is merging the changes that implement the ZooKeeper connection requirement from every Zuul component. Currently the changes that implement those also include some extra changes that refactor the ZooKeeper connection classes. I think that's a good idea, but I'd like to separate concerns and focus on getting the requirements for the 4.0 release taken care of first. On Friday I discussed with some folks an approach to that, and today I finished up a rebase + rework of the series. I uploaded three new versions of these patches: https://review.opendev.org/775459 Mandatory Zookeeper connection for ZuulWeb https://review.opendev.org/775460 Connect executor to Zookeeper https://review.opendev.org/775461 Connect merger to Zookeeper I think they represent the minimum we need to get into 4.0. There is some boilerplate related to ZK connections in each service, but it's easy to reason about for now. I do think we should refactor the ZK connection class, but let's do that on top of these patches. The results will be clear and easy to understand. The original patches also increased the unit test job timeout because the jobs took approximately 25% longer to run. I think perhaps reviewers were under the impression that was necessary due to the new ZooKeeper connections in unit tests. I investigated the issue and found that was not the case. The actual cause was due to extra import statements that were added to support the addition of type annotations. After removing those type annotations and associated import statements, the test runtime remains the same as it currently is. You can read more details about it in the comments of patchset 2 of the last change. It's also worth keeping in mind that extraneous import statements can cause not only unanticipated problems like this, but can often exacerbate the problem of import loops, something we have seen occasionally in Zuul. Keeping additional imports for type annotations to a minimum will be beneficial. I propose the following next steps: 1) Re-propose 775459-775460 as new patchsets on their original changes in order to preserve the review history there. 2) Merge those changes and continue work on releasing 4.0. 3) Meanwhile: refactor the ZooKeeper connection class. 4) Continue work on the remaining scale-out scheduler changes. -Jim

1 0

Nodepool image building
by jim＠acmegating.com 11 Feb '21

11 Feb '21

Hi, I'd like to propose some new features related to Nodepool and image building. The short version is that right now image management is tied fairly closely to diskimage-builder within the nodepool-builder process. This is problematic for folks who want Nodepool image management but can't use DIB (ie, Windows images). It's even problematic for those who do want to use DIB, in that it's difficult to debug image builds. It's worth noting the original Zuulv3 spec had room for more than one nodepool-builder implementation (in an offhand comment, it mentioned that a snapshot-based nodepool-builder process could be created). However, that has not yet come to pass. Over the years, we've speculated about the idea of using Zuul to build images, and I think it's time to see if we can put that into practice. It makes a lot of sense in that we have a ready-made execution environment with Zuul -- build logs and history are well defined and accessible to anyone. It's flexible and can accommodate any image building system (DIB or otherwise). The main downside is the inception aspect, but I'm not worried about that (more later). I think the key requirements are: * Able to create images using any tooling. * Visibility into build history. * Nodepool-launcher should automatically use the "latest" build. Part of this solution is obvious, but I think there are open questions for other parts: * Should the Zuul job be responsible for uploading the image to the cloud? * Should Nodepool be responsible for deleting images? About that downside: clearly there's an issue when bootstrapping a new Zuul system -- how can you run jobs without images? Most new Zuul installations start by using cloud images from the provider. Any Zuul user who wants custom images likely already has some kind of job running on some kind of image anyway. I think it's a natural progression to create a Zuul job to build a custom image and then begin using that custom image in "real" Zuul jobs. Long story short: bootstrap the system with cloud-images. As an OpenDev admin, I think I would be comfortable with a Zuul job that built our DIB images using cloud provider images -- I don't think the differences between those images are so great as to make a DIB job difficult to write. However, if we did use this system, I'm not sure we would ever have to. We have enough clouds with our own images in them already, we could probably use our images to make new ones. In this case: a chicken with no egg. :) Let's work through how this might look. First the easy part, then I'm going to propose some alternate solutions. 1) Build the image: A Zuul job ("build-nodepool-image-ubuntu-focal") is triggered by a timer. It has a mutex to avoid duplicate image builds. It runs DIB in this case but could just as easily run packer or even snapshot the system it's running on (we can add roles in zuul-jobs for common image building systems). 2) Get the image into the cloud(s): a) Our first option is to do this work in Zuul. Have the job (or a dependent job) upload the image to the cloud(s) (if this was a snapshot build, that's a no-op -- it's already there). Advantage: the upload log is visible along with the build log. Disadvantage: if the upload fails, we won't retry like Nodepool does. b) An alternative would be to have Nodepool do this. Nodepool could even query the Zuul API to find the latest build of build-nodepool-image-ubuntu-focal, check the artifacts list, and download the image file from there. After that, it looks a lot like the current nodepool-builder process right after DIB runs (it has a file on disk and it starts to upload it). Advantage: we use all of the Nodepool image lifecycle management. Disadvantage: upload logs are hidden from users (like today). to integrate snapshot builds, we would have to download the snapshot and re-upload it. 3) Get nodepool-launcher to use the new image: a) If we use 2.a, then we have an image in the cloud without a record in Zookeeper. i) We could have nodepool-builder (or something like it) scan the cloud for images matching a certain pattern and create ZK records for them. ii) Or we could just have nodepool-launcher look for that pattern and use the latest image. b) If we use 2b, then launcher will work as it does today with nodepool-builder; no changes are needed. 4) Delete the image when it's no longer needed We should use the n-2 rule we have today so there's always at least 2 images in the cloud for easy rollback. a) If we used 2.a + 3.a.iii then we don't have ZK records. It would probably make sense to handle image deletes in Zuul jobs with this setup. Presumably the jobs would query the cloud API, list images, and delete them according to the age rules. The disadvantage here is we'd be re-implementing a good chuck of nodepool-builder in Ansible modules. It's doable, but may be a bit clunky. b) If we use 2.a + 3.a.i or 2.b + 3.b then we have records in ZK, so nodepool-builder (or something like it) can delete the images more or less how it does today. I think those are most of the choices. Now that I've typed all of that out, I think I really like the system with "b" for all the choices. I'm not too worried about the upload logs being hidden. And the snapshot system is workable, if not ideal. If we really wanted to support snapshots better, then 2.a + 3.a.i + 4.b would probably be best, but at the cost of moving the upload code into zuul-jobs roles. Or we could do the same thing we did last time and implement the "b" solution ("nodepool-zuul-builder") and if someone really wants to do snapshots, then we implement 2.a[snapshot] + 3.a.i + 4.b as ("nodepool-snapshot-builder"). That would probably be fairly efficient as far as code goes. Finally, a word about DIB and the current nodepool-builder: none of this requires that we change anything about the current nodepool-builder process. If we like it, we can keep it exactly as it is. Indeed, in OpenDev's case, we rely very heavily on the cache on the build host, so it's not at all clear that doing builds on throwaway nodes is ideal. Of course, we could do builds on static Zuul nodes and get the benefit of the cache. I think the way to approach this would be to implement the new system, then try using it with DIB, and if we love it and find no significant drawbacks, consider deprecating the current nodepool-builder. If we don't love it, we'll keep it. I'd appreciate feedback on this. I know this is a bit free-form; I'll take the feedback I get and synthesize it into a proper spec. -Jim PS: there's a fairly well-known hack for making non-DIB image builds by replacing DIB with a script that accepts the same arguments. The "b" plan could actually be implemented today with no changes to Nodepool using that same hack. A final implementation probably wouldn't look much different structurally.

5 6

job.provides/requires for periodic pipeline
by Vitaliy Lotorev 11 Feb '21

11 Feb '21

Hi, I have a jobA for project A (job builds images) and jobB for project B (job runs tests for those images). I set both jobs into a nightly periodic pipeline. periodic pipeline has jitter ±5 minutes. jobA and jobB run in pipeline asynchronously. jobB doesn't wait till jobA finishes and fetches latest successful build from jobA which appears to be built 24 hours ago (i.e. jobB tests in images built 24 hours ago). Questions: * Would setting jobA.provides=images and jobB.requires=images order them during one run? I.e both jobA and jobB would enqueue and jobB would wait while jobA finishes. Or should I use job.dependencies? * What are other assumptions for using requires/provides: ** Should be jobs in the same pipeline? ** if job has 'requires' how the job can be debugged separately without running jobs that define 'provides'? -- Regards, Vitaliy

2 3

Type annotations
by Jan.Kubovy＠bmw.de 09 Feb '21

09 Feb '21

…. > I'm not [yet] an expert on type annotations in Python, so I may have to > rely on others who know more details on what we can and should do in the > future, but here are some guidelines that might be a good starting > point: > > * Let's not bend over backwards to make mypy happy. That's not our > goal, so if something is tripping it up but is otherwise not ambiguous > to humans or IDEs, do the minimum work to ignore it. > > * Corollary: do we actually need/want to keep running mypy > automatically in our Zuul jobs? > > * Let's focus on improving interfaces, but not exhaustively typing every > variable. > > * If there are improvements coming in the future (like 3.9) let's do > less now and wait for them. > > And based on those, what do you think about these specific guidelines: > > * Do type method signatures. This is always a good idea without it most of the checking will not work. > * Don't recurse ("List[Callable]", not "List[Callable[[], None]]"). I would not make that a hard rule, more a recommendation. If there Is a benefit in certain cases I would allow it. One can also use type aliases and type vars to make it better usable and look less awkward. > * Avoid typing variable assignment inside of methods. (They should > mostly be inferred, and they do little to improve code comprehension.) If this works reliably, for everyone using it (not sure if this will not break for example usage search). Otherwise I don’t see as much drawback having a list of typed assignments in the init(). But I guess this Is a matter of taste. Jan _______________________________________________ Zuul-discuss mailing list Zuul-discuss(a)lists.zuul-ci.org<mailto:Zuul-discuss@lists.zuul-ci.org> http://lists.zuul-ci.org/cgi-bin/mailman/listinfo/zuul-discuss

2 1

Type annotations
by Jan.Kubovy＠bmw.de 08 Feb '21

08 Feb '21

Hi all, my 0.02$ to the discussion: Although I am currently not an active contributor nor a reviewer a year ago I started the work on scale-out-scheduler as a part of my day job. This lead me to introduce the typings in parts of old code and use it in new code. As zuul is a community project I do believe that it relies not only on core developers, who are involved over long period of time but should also be code-friendly to contributors, who want to or are tasked to one-time, short-period jobs. While I do like dynamic languages and understand their advantages, I worked with e.g. with Ruby a decade and half ago and I work with python in other projects too, I do like more an as-complex-as-necessary and as-simple- and as-readable-as-possible code base. IMHO it always depends on a context: A small POC would probably never use type annotation, a huge code base like zuul should IMHO consider it. I do agree that typings add a certain degree verbosity (I was an early adopter of Kotlin - one of the reasons was reduced verbosity, optional type defs) but they also come with certain benefits - better code navigation, support when refactoring, IDE support, etc. Also, I do not understand use of type annotations as an attempt to make python statically typed. The issues/reasons which lead me to introduce them: - Ambiguous parameter/argument names E.g. project, tenant, job, to name a few. Sometimes a name, sometimes an object, sometimes an Ansible Job, sometimes a zuul job or build. Makes it hard to navigate in the code. Since some of those names are too generic it becomes a challenge when working on the heart of zuul. Probably not so much when dealing with small changes. Just to be clear I am not talking about cases here where one may intentionally want to pass an "project" argument which may be both, a project name and a project object, and the code can deal with both. Those occasions barely exist here. Usually it is one or the other. - NPE, missing property Setters called right after instantiating an object. Making the argument basically optional. Since scale-out-scheduler touches a lot of code I encountered more than once that something which supposed not to be none was after a bit of refactoring. Just by navigating in the code was not always easy to see where a certain argument is set. E.g. try to find usages of attribute job, project, etc and where those are set for certain object. You will get every class containing that attribute - basically, reducing it to a text search result. This (https://review.opendev.org/c/zuul/zuul/+/757149/14/zuul/scheduler.py) is an example where one can see a simplification of code and reducing the risk of runtime error where typings were involved. Tests are great but the theoretical case that someone may set some attribute to None (a new change or config case) may not be covered (I guess zuul tests do not cover 100% branches/complexity). While it is not bulletproof typings do help here. - Code Navigation IDE support, usage search, etc. - API changes and refactoring Was, and I believe still is, very much needed especially in big tasks like scale-out-scheduler. - Code documentation This goes a bit against the verbosity argument. When the type is actually documented in the comment section making it in addition prone to inconsistency. Type systems additionally help me to prevent some errors and make me thing time to time if my concepts, implementation is correct. I did make myself as the first guinea pig to test the efforts vs. benefit argument in zuul and it did serve me the purpose. After more colleagues were involved in that topic, we had the same discussion and came eventually to the same result. I understand there is an effort needed, but I also believe that the benefits outweigh the cost dramatically, especially in long-term and when considering a need-to usage. That said, I never thought about a 100% typing through the code base. For example, some classes, e.g. zuul.cmd.Scheduler may not benefit from typings as much, classes in zuul.model definitely would, due to their reuse thought-out the code. For the juniority argument I strongly believe that also experienced developers may find this beneficial especially in cases described above (one-time, short-term involvement). Unless there is, of course, an argument against such contributions (I am not aware of). Since, typings are now part of python and evolving over the past few versions, it does not seem to go away any time soon. From what I can see more and more pyhon code uses them. I do not agree that using typings is a sign of choosing the wrong language here. There may be use-cases where not using them may be more beneficial (fast prototyping, etc), and some use-cases where using them will benefit the project. Also using typings does not prevent loose/dynamic typing (e.g. Union or just not using them everywhere if it makes sense). It would be pity not to use the chance of a big change like scale-out-scheduler/zuul 4.x is to improve the code just to find ourselves few years down the road regretting it. The trend suggests that having the option of type hinting (while keeping dynamic types) is the way to go since it kind of offers the benefits of both worlds - other languages e.g. TypeScript, Kotlin JS, to name a few. Since, I felt this will be a controversial change (changes always are), I tried to keep the typings in a separate change in gerrit were ever I could - so you can easily remove them by abandoning those changes. They served me well and should I get another task in zuul I would like to find them again also in other parts of the code. Cheers Jan _______________________________________________ Zuul-discuss mailing list Zuul-discuss(a)lists.zuul-ci.org<mailto:Zuul-discuss@lists.zuul-ci.org> http://lists.zuul-ci.org/cgi-bin/mailman/listinfo/zuul-discuss

1 0

Making gerrit zuul result comments more readable
by Sorin Sbarnea 02 Feb '21

02 Feb '21

At this moment the comments added by zuul on gerrit can be very hard to read, partially due to gerrit limitations I have projects where I cannot even fit them in a single screen! -- each job spans **3** rows instead of a single one. https://sbarnea.com/ss/Screen-Shot-2021-02-02-09-04-45.40.png While my hopes on adding markdown support on gerrit comments <https://bugs.chromium.org/p/gerrit/issues/detail?id=9299> are low, as nobody from google even did bother to get involved. (please comment/star ticket if you want it) I know that we plan to integrate the results in the UI and avoid comments, but that is long temp plan which does not fix the issue for email notifications at all. Here are some measures that we could do to lower the amount of wasted space: a) Put an emoji stating the result as the first item in the list, which should make it easy to spot the failed ones ❌ foo ... ✅ bar ... b) sort jobs alphabetically c) remove seconds from duration, they are useless even for the fastest jobs d) use shortened build-id for URLs, 5 chars should provide more than enough entropy If we do all of these we should make it fit on a single line, even for longer job names. Still, every single one would add visible benefits. What do you think? I also considered other alternatives like listing failed at top and non-voting at bottom, so the order will be failed, passed, non-voting, but I can see how this may struggle to be accepted. The root cause of this formatting issue is this style: .message { --gr-formatted-text-prose-max-width: 80ch; } Sadly due to the use of shadow-dom, I can no longer use css styling to override the stylesheet. -- /sorin

2 1