| *** Goneri has quit IRC | 00:08 | |
| *** rlandy has quit IRC | 00:12 | |
| *** cloudnull has quit IRC | 00:23 | |
| *** cloudnull has joined #zuul | 00:24 | |
| *** Goneri has joined #zuul | 00:49 | |
| *** Goneri has quit IRC | 01:03 | |
| *** swest has quit IRC | 01:16 | |
| *** threestrands has joined #zuul | 01:19 | |
| *** swest has joined #zuul | 01:30 | |
| *** dennis_effa has joined #zuul | 01:44 | |
| *** saneax_AFK has joined #zuul | 02:14 | |
| *** rfolco|rover has joined #zuul | 02:41 | |
| *** saneax_AFK has quit IRC | 02:49 | |
| *** bhavikdbavishi has joined #zuul | 03:03 | |
| *** bhavikdbavishi1 has joined #zuul | 03:07 | |
| *** bhavikdbavishi has quit IRC | 03:09 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 03:09 | |
| *** bhavikdbavishi has quit IRC | 04:20 | |
| *** bhavikdbavishi has joined #zuul | 04:20 | |
| *** bhavikdbavishi1 has joined #zuul | 04:23 | |
| *** dennis_effa has quit IRC | 04:24 | |
| *** bhavikdbavishi has quit IRC | 04:24 | |
| *** bhavikdbavishi1 is now known as bhavikdbavishi | 04:24 | |
| *** evrardjp has quit IRC | 04:33 | |
| *** evrardjp has joined #zuul | 04:33 | |
| *** sgw has quit IRC | 04:34 | |
| *** y2kenny has quit IRC | 05:14 | |
| *** saneax_AFK has joined #zuul | 06:22 | |
| *** saneax_AFK is now known as saneax_ | 07:00 | |
| *** hashar has joined #zuul | 07:04 | |
| *** jcapitao has joined #zuul | 07:08 | |
| *** iurygregory has quit IRC | 07:11 | |
| *** rpittau|afk is now known as rpittau | 07:21 | |
| *** tosky has joined #zuul | 07:29 | |
| *** iurygregory has joined #zuul | 07:33 | |
| *** bhavikdbavishi has quit IRC | 07:34 | |
| *** dustinc has quit IRC | 07:35 | |
| *** jpena|off is now known as jpena | 07:56 | |
| *** bhavikdbavishi has joined #zuul | 08:00 | |
| *** bhavikdbavishi has quit IRC | 08:46 | |
| *** bhavikdbavishi has joined #zuul | 08:47 | |
| *** hashar has quit IRC | 09:25 | |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 09:27 |
|---|---|---|
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 09:28 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 09:43 |
| *** ysandeep is now known as ysandeep|lunch | 09:48 | |
| *** wuchunyang has joined #zuul | 09:51 | |
| *** sshnaidm|afk is now known as sshnaidm | 10:14 | |
| mhu | hello tobiash, could I have reviews on https://review.opendev.org/#/q/status:open+project:zuul/zuul+branch:master+topic:CORS_OPTIONS (needed for UI dev) please? | 10:17 |
| *** rpittau is now known as rpittau|bbl | 10:19 | |
| *** ysandeep|lunch is now known as ysandeep | 10:20 | |
| openstackgerrit | Carlos Goncalves proposed zuul/zuul-jobs master: configure-mirrors: add CentOS 8 Stream https://review.opendev.org/734787 | 10:27 |
| *** wuchunyang has joined #zuul | 10:30 | |
| *** wuchunyang has quit IRC | 10:36 | |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Update access token url https://review.opendev.org/734793 | 10:48 |
| *** jcapitao is now known as jcapitao_lunch | 10:56 | |
| *** bhavikdbavishi has quit IRC | 11:00 | |
| *** bhavikdbavishi has joined #zuul | 11:01 | |
| *** threestrands has quit IRC | 11:16 | |
| openstackgerrit | Guillaume Chauvel proposed zuul/zuul master: zuul_unreachable: Fix ansible callback exception https://review.opendev.org/734797 | 11:19 |
| *** jpena is now known as jpena|lunch | 11:40 | |
| *** bhavikdbavishi has quit IRC | 11:50 | |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 12:05 |
| *** rpittau|bbl is now known as rpittau | 12:05 | |
| *** bhavikdbavishi has joined #zuul | 12:07 | |
| *** rlandy has joined #zuul | 12:18 | |
| *** jcapitao_lunch is now known as jcapitao | 12:20 | |
| mordred | tobiash: you have angered the indentation gods | 12:21 |
| tobiash | mordred: too easy to anger them... | 12:22 |
| fungi | i sacrifice my spacebar to them, in hopes of a more fruitful coding season | 12:23 |
| openstackgerrit | Tobias Henkel proposed zuul/zuul master: Update access token url https://review.opendev.org/734793 | 12:23 |
| mordred | yah. the error in that patch is why I've stopped using visual indent in my code and always do a line break after a ( and a 4-space indent on the next line - it saves carnage in simple patches like that | 12:23 |
| mordred | yup. just like that ) | 12:24 |
| mordred | :) | 12:24 |
| tobiash | :) | 12:24 |
| fungi | sometimes it wants 8 space indent, if the line which would come after needs semantic indentation | 12:24 |
| fungi | so even for those who eschew visual indent, the rites of the indentation gods are not trivial | 12:25 |
| *** olaph has quit IRC | 12:28 | |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 12:36 |
| *** jpena|lunch is now known as jpena | 12:38 | |
| openstackgerrit | Sagi Shnaidman proposed zuul/zuul-jobs master: Add jobs for testing ensure-ansible https://review.opendev.org/734584 | 12:40 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 12:46 |
| *** hashar has joined #zuul | 12:46 | |
| *** Goneri has joined #zuul | 12:48 | |
| tobiash | mhu: done | 12:49 |
| mordred | fungi: I've actually adopted the black approach: https://review.opendev.org/#/c/734810/1/plugins/modules/server.py@749 | 12:53 |
| mordred | fungi: no need for occasional 8-space | 12:54 |
| *** rlandy_ has joined #zuul | 12:54 | |
| *** harrymichal has joined #zuul | 12:56 | |
| *** rlandy has quit IRC | 12:58 | |
| *** rlandy_ is now known as rlandy | 12:59 | |
| fungi | blasphemer | 12:59 |
| *** sgw has joined #zuul | 13:03 | |
| openstackgerrit | Sagi Shnaidman proposed zuul/zuul-jobs master: Add jobs for testing ensure-ansible https://review.opendev.org/734584 | 13:05 |
| openstackgerrit | Sorin Sbarnea (zbr) proposed zuul/zuul master: Enable display of failures inside zuul console https://review.opendev.org/734833 | 13:18 |
| avass | any zuul maintainer that wants to take a look at: https://review.opendev.org/#/c/722478/ ? pretty small change that adds some information to the mqtt driver | 13:45 |
| zbr | avass: fungi: mordred: take a look at ^ -- finally making "failures" visible in the dashboard. | 13:49 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 13:55 |
| avass | zbr: that sounds like a pretty good feature actually | 13:56 |
| zbr | tell me... i was driven by pain, not desire to polish the ui. | 13:56 |
| zbr | pain is a great motivator | 13:57 |
| openstackgerrit | Matthieu Huin proposed zuul/zuul master: web UI: allow a privileged user to dequeue a change https://review.opendev.org/734850 | 13:58 |
| openstackgerrit | Sagi Shnaidman proposed zuul/zuul-jobs master: Add jobs for testing ensure-ansible https://review.opendev.org/734584 | 14:04 |
| *** rlandy is now known as rlandy|mtg | 14:08 | |
| *** bhavikdbavishi has quit IRC | 14:09 | |
| *** sshnaidm is now known as sshnaidm|bbl | 14:23 | |
| *** rlandy|mtg is now known as rlandy | 14:31 | |
| *** hashar has quit IRC | 14:39 | |
| corvus | zbr: are you familiar with clicking the "FAILED" button on the console output? | 14:57 |
| *** bhavikdbavishi has joined #zuul | 14:57 | |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: gitlab - add the merge request updated test https://review.opendev.org/734869 | 15:00 |
| zbr | corvus: tbh, i am not usually using it. | 15:02 |
| zbr | i bet few users know that they can click the status labels to get more details | 15:02 |
| corvus | zbr: i ask because in your commit message you said that users were "forced" to look in the json file, but in actuality, all of the non-standard additional attributes (like "failure") are available there, so i thought maybe you didn't know about it. | 15:02 |
| zbr | i also find confusing the fact that we have two different types of expansion: one per-line, and one per label inside that line. | 15:03 |
| corvus | zbr: it's a ui paradigm zuul shares with ara, so i thought it might be familiar to folks | 15:03 |
| corvus | maybe we could do something to make it more obvious | 15:03 |
| zbr | each of them with different levels of expansion | 15:03 |
| zbr | i can rewrite the commit message to remove the "forced part" | 15:03 |
| corvus | zbr: i don't quite follow the part about types of expansion; can you help me find what you're talking about? | 15:04 |
| zbr | ok, let see one simple example https://review.rdoproject.org/zuul/build/8273b553847e4f0eba3caf6992ef1878/console | 15:04 |
| dmsimard | yeah clicking on the status in ara was reported to be non-obvious for certain users | 15:04 |
| zbr | if you chick the 2nd blue line, it will expand/contract | 15:04 |
| corvus | zbr: by 2nd blue line, you mean "Install docker-ce"? | 15:05 |
| zbr | label (from inside the same clickable area) opens a popup instead. | 15:05 |
| zbr | yep | 15:05 |
| zbr | nesting clickable areas are not really very user friendly | 15:05 |
| corvus | i'm not sure i agree with that as a general rule. but would you prefer to only have the caret clickable for expansion? | 15:06 |
| avass | corvus: I had no idea you could do that :) | 15:06 |
| corvus | okay, so let's fix the problem that clicking the detail button is not obvious :) | 15:07 |
| avass | yeah | 15:07 |
| zbr | i prefer the expandable version for multiple reasons: clickable are much wider, no motion sickness like on popup, not intrusive (can stills croll the page) | 15:07 |
| zbr | i find the popup intrusive in multiple ways | 15:07 |
| corvus | i would rather do that before adding more non-standard output fields | 15:07 |
| zbr | failures is pretty much standard, is not a "custom module" | 15:08 |
| dmsimard | zbr: historically the popup in ara 0.x was a huge hack that allowed for asynchronous fetching of results so it wouldn't all be rendered in a single (endlessly loading) page | 15:08 |
| dmsimard | it's literally a modal and an iframe | 15:08 |
| corvus | what's displayed is intended to be close to what you get on stdout/stderr when you just run ansible; then the detail button is there to show you all the extra info | 15:08 |
| corvus | oy. it looks like the package module behaves differently for different operating systems | 15:09 |
| zbr | guess what: ansible does display the failures on console too, stdout/stderr is something specific to process executions, but modules that run using API will likely not produce output on these. | 15:09 |
| corvus | under debuntu, all of the error information is in 'msg' and there is no 'failures' attribute. but under fedora, 'msg' is a general error message, and the specific package failure is under 'failures'. | 15:09 |
| zbr | i did not investigate much, but my impression is that "failures" is not unique to package module. | 15:10 |
| corvus | is that just because failed tasks output the entire returned structure? | 15:11 |
| corvus | fatal: [localhost]: FAILED! => {"changed": false, "failures": ["No package aoeuaoueo available."], "msg": "Failed to install some of the specified packages", "rc": 1, "results": []} | 15:11 |
| zbr | i am sure that "failures" is a critical information that should be displayed with priority. | 15:11 |
| *** dustinc has joined #zuul | 15:11 | |
| corvus | that's what i see when running a test playbook | 15:11 |
| avass | corvus: isn't 'package' just supposed to be a wrapper for yum/apt etc so I'm guessing it's just calling those modules and they behave differently | 15:12 |
| zbr | an considering we would display it only when present, i do not see any issue. | 15:12 |
| corvus | avass: yeah, though i wonder what's different in the yum case for it to add an extra data return channel | 15:13 |
| zbr | i did a grep on ansible source code, there is huge number of modules that return "failures". | 15:13 |
| corvus | zbr: can you point me at one other? | 15:14 |
| dmsimard | any module can return any fields, there's little to no standard :( | 15:16 |
| corvus | yeah, this is why getting folks to understand they can click on the details button is important | 15:17 |
| corvus | dmsimard: do you have any thoughts about improving that? | 15:17 |
| corvus | only thing i'm coming up with right now is maybe a little banner at the top of the page letting folks know about it... | 15:17 |
| corvus | or maybe don't overload the status field, and just add a "Details..." link to each row | 15:18 |
| corvus | i do like how the current system avoids clutter, but if people aren't finding it then maybe it's worth a little clutter | 15:19 |
| zbr | i was wrong, only dnf module returns failures (other usages of failures variable do not return them) | 15:21 |
| zbr | this may explain the lack of interest in addressing this issue | 15:22 |
| dmsimard | corvus: I don't have a strong opinion but I think it makes sense to iterate through a number of fields we know about and think might be relevant to the users and display them inline -- the main issue with this being that modules can return anything they want | 15:23 |
| avass | corvus: looks like dnf is logging any package that couldn't be installed in failures and logging installed packages in results | 15:24 |
| corvus | dmsimard: hrm, i kind of want to lean into ansible's paradigm here -- that we can't know what fields will be useful, so we should make it easy for users to see them | 15:24 |
| avass | yum, apt, and dnf looks like they're behaving very differently and that's why :) | 15:26 |
| dmsimard | corvus: so then next best thing is to show all the fields inline ? would be more cluttered but I guess it's not that bad because only the failed tasks are expanded | 15:27 |
| corvus | the easiest thing to do would be to just do what the stdout plugin does and output everything on error, but we're not in a serial environment, we actually have interactive structured data. it's really rich, so let's use it. :) | 15:27 |
| corvus | dmsimard: no i don't want to do that | 15:27 |
| corvus | i want people to click on the details button | 15:27 |
| corvus | if the task didn't output enough information to stdout/stderr, the next thought for the user should be to dig deeper and see all the returned data to aid in debugging | 15:28 |
| corvus | so how do we get users to that point? | 15:28 |
| dmsimard | UI/UX is hard :) | 15:29 |
| zbr | corvus: can you show me one real-world example where adding failures would lower the UX? | 15:29 |
| corvus | yes, which is why it's fun and rewarding :) | 15:29 |
| corvus | zbr: no, because there is only exactly one case where it appears | 15:30 |
| corvus | but what's the next interesting key we should add? | 15:30 |
| avass | how about instead of the button saying 'failure', 'changed' etc we could color code it and it could always say 'details' | 15:30 |
| corvus | are we going to evaluate each one on a case-by-case basis? | 15:30 |
| *** ysandeep is now known as ysandeep|away | 15:30 | |
| avass | or something like that | 15:30 |
| zbr | if we would not be talking about a core module, i would have said that the module should be updated. but is core and one of the top 10 most used ones. | 15:31 |
| corvus | doesn't that argue all the more that it should be updated | 15:31 |
| zbr | i can try to make PR to ansible to fix it, but even if approved, it will happen only on future versions, nothing that we would use by default for at least one more year from now. | 15:32 |
| corvus | avass: for accessibility purposes, it's important not to rely only on colors as a signal. colors + words is usually best. | 15:33 |
| avass | right | 15:33 |
| corvus | so i think if we want the word "details" to show up there, it'll have to be a new field | 15:34 |
| *** yoctozepto has quit IRC | 15:34 | |
| *** yoctozepto has joined #zuul | 15:35 | |
| corvus | i wonder if we could do a toast message the first time someone visits a build page that says "click the result to see more details".... | 15:35 |
| corvus | or we could put a (?) icon next to the result -- maybe that would be obvious enough but not as cluttered as a new details button? | 15:36 |
| avass | I was thinking about the (?) or some sort of icon | 15:36 |
| clarkb | binoculars | 15:37 |
| corvus | better | 15:37 |
| corvus | "FAILED (oo)" is better than "FAILED (?)" :) | 15:37 |
| corvus | (did it fail? click to find out!) | 15:37 |
| avass | I was thinking either a magnifying glass or a document icon | 15:38 |
| corvus | also better than (?) :) | 15:38 |
| avass | but yeah something like that | 15:39 |
| avass | I'd rather avoid one-time toast messages :) | 15:39 |
| mhu | "did it fail ? the results will SHOCK you!" | 15:40 |
| avass | "TEN REASONS this build FAILED" | 15:40 |
| zbr | lets first fix the failures issue in particular and evaluate the expand vs collapse issue later, as it would need proper user feedback | 15:40 |
| zbr | we cannot rely only on own own experience when making UX decisions on zuul. | 15:41 |
| corvus | ha :) avass, yes, i think the one-time toast may be too annoying. though i am starting to think that having a little "intro" on the main build landing page may not be a bad idea | 15:41 |
| zbr | i have some ideas/theories, but without a user survey is pure speculation | 15:41 |
| corvus | zbr: we can do the best we can. | 15:41 |
| zbr | imho: we should have only one way to show more details on a task, the expansion one. but i can understand that may be oppionated. | 15:42 |
| corvus | i agree proper ux testing is great. but we also definitely don't do this in a vacuum. everything we do is heavily influenced by user feedback. | 15:43 |
| corvus | zbr: i see the attraction there, but that doesn't scale well in the case of some tasks which return massive amounts of data | 15:44 |
| corvus | https://www.patternfly.org/v3/styles/icons/index.html | 15:44 |
| zbr | the fact that detail section is expanded by default makes it more accesible: user does not have to do anything to see it (as opposed to the popup) | 15:44 |
| corvus | pficon-search is a magnifying glass | 15:44 |
| zbr | maybe we can have the expansion at two levels, implicit one (like now, with cherry picked fields), and an all fields one. | 15:45 |
| corvus | zbr: how is that different? | 15:46 |
| corvus | zbr: do you just mean: get rid of the modal popup? | 15:46 |
| zbr | not to clutter the interface with stuff that is unlikely to be useful? | 15:46 |
| avass | fa-plus could also work | 15:47 |
| zbr | i would not mind ditching the modal, especially if we include the same information in expand. | 15:47 |
| corvus | zbr: the current state is: implicit with cherry-picked fields, and the modal popup is all fields. so what's new in the proposal? just folding the popup into the main page? | 15:47 |
| zbr | corvus: yep | 15:47 |
| zbr | corvus: i think you described it well, folding the information from modal into the main page (keeping its dynamic implementation) | 15:48 |
| corvus | zbr: so to summarize your proposal, it would be that when users land on the console tab, it looks mostly like it does now, except that instead of clicking on the result to open a modal popup, we would add something to the expanded area, so to get to details, a user would expand the task first, then click a button inside the expanded area which adds all of the fields into the expanded area. that button would | 15:49 |
| corvus | be a toggle, and clicking it again would reduce it to the cherry-picked fields. that sound about right? | 15:49 |
| zbr | yep, something very similar with the more/less implementation i did for stdout/stderr | 15:50 |
| zbr | summary/details html elements are quite handy here. | 15:50 |
| corvus | that sounds like an improved ux for dealing with failed tasks, though if someone wants to see details for successful tasks (which aren't expanded by default), we're adding a click to that workflow. i use that quite often (i frequently need to see the results of successful tasks, in order to find the inputs for failed tasks later). | 15:51 |
| zbr | summary contains the few notable fields, and details contains the rest (collapsed by default) | 15:52 |
| corvus | perhaps the more/less button could be on the collapsed task bar, so that if you click that, it auto-expands. basically it would look identical to the "details" magnifying glass or whatever, but would not pop-up the modal, but rather fully expand the inline view. | 15:53 |
| zbr | corvus: are you sure that the limited set does not help you with successful tasks? what percent of those would require an extra click? | 15:53 |
| corvus | zbr: i'm usually looking for list entries | 15:53 |
| corvus | like "what were the with_" items for a task | 15:54 |
| zbr | i am sure we could avoid two clicks for succesful tasks easily, but i am not sure about benefits, i need to look. | 15:54 |
| zbr | anyway, it would be under our control to decide which is better | 15:54 |
| zbr | sometimes it needs few iterations to tune it | 15:55 |
| zbr | what would be extreamly useful for improving the dashboard experience would be to have pre-generated job result as a model. | 15:56 |
| zbr | now we need to dig for examples of jobs that can exemplify the changed behavior | 15:56 |
| zbr | also we cannot share links to these | 15:56 |
| zbr | if we would build a static one, it would make it much easier to get feedback from average users (not zuul contributors) | 15:57 |
| *** bhavikdbavishi has quit IRC | 15:59 | |
| corvus | i'd like to summarize where i stand: on zbr's patch to whitelist 'failures': i'm 45% in favor of it. i would rather avoid it because i worry that if we add this one exception, people will start to expect more and more fields included by default. yes, that's a slippery slope argument. however, if other zuul maintainers think it's worth working around this niche use case in ansible (package module using | 16:01 |
| corvus | dnf doesn't return enough info) to special case it, i won't -2 it. but the change does need some updates (i left inline comments). i feel strongly that we should address the fact that the details link lacks an affordance (ie, it is not obvious to new users that it's available and what it does). i think moving that to an icon should be easy and quick, and a non-intrusive change, and we should do it asap. | 16:01 |
| corvus | i think zbr's suggestion of folding the modal into the inline is worth considering, and after we've moved the details button to a dedicated icon, we'll have a bit more information about how we might want to set that up. i'm not convinced we'll want to do it, but i'd rate my likelihood of approving something like that at 70% at this point, just based on imagining it. | 16:01 |
| corvus | zbr: if we change the reported preview link to use zuul-preview, we may be able to get sharable deep-links into preview builds. | 16:02 |
| avass | there doesn't seem to be enough modules that use | 16:03 |
| avass | 'failures' when looking at the ansible repositories to motivate whitelisting it | 16:04 |
| *** rpittau is now known as rpittau|afk | 16:07 | |
| *** bhavikdbavishi has joined #zuul | 16:11 | |
| zbr | avass: dnf module affects anyone on rpm platforms, it is quite a common source of failures, sic. | 16:11 |
| zbr | i can obviously change the implementation to make it work even with non list results, that is not the issue. | 16:11 |
| openstackgerrit | Fabien Boucher proposed zuul/zuul master: gitlab - add the merge request updated test https://review.opendev.org/734869 | 16:13 |
| fungi | it seems like either the dnf module should seek alignment with other core modules, or ansible should bless the mechanism and data structure used by the dnf module as a standard and start encouraging other modules to use it | 16:16 |
| zbr | fungi: what about modules that can return multiple errors? msg is clearly a single string afaik. | 16:20 |
| zbr | ansible team is these days deep into other more serious issues, so i doubt we would be able to get any useful reply on that. | 16:20 |
| zbr | cd me | 16:21 |
| *** jcapitao has quit IRC | 16:21 | |
| fungi | it seems like richer data structures for returned errors is something they might welcome a spec for | 16:21 |
| *** dennis_effa has joined #zuul | 16:23 | |
| zbr | lets test the water on #ansible-devel ... | 16:26 |
| openstackgerrit | Matthieu Huin proposed zuul/zuul master: web UI: user login with OpenID Connect https://review.opendev.org/734082 | 16:55 |
| openstackgerrit | James E. Blair proposed zuul/zuul-jobs master: Allow upload-docker-image role to be used outside of promote https://review.opendev.org/734890 | 16:56 |
| corvus | mhu: what determines the value of signinRedirect() ? | 16:58 |
| corvus | mhu: is it this? redirect_uri: API.getHomepageUrl() + 't/'+ tenant + '/auth_callback', | 16:59 |
| mhu | corvus, the redirect URI you mean? yes, it's from the userManager config | 16:59 |
| mhu | so, the thing is, usually when you add redux-oidc to your project, you're going to use a single realm with it | 17:01 |
| mhu | so you can instantiate your userManager nicely at the root of your app with a config file somewhere | 17:01 |
| *** jpena is now known as jpena|off | 17:01 | |
| mhu | but in our case, and most likely vexxhost's case too, we'd want to associate a realm to a tenant | 17:02 |
| mhu | so I had to do this little dance where I get the IdP info for a realm from the info API endpoint, and instantiate the userManager on the fly | 17:03 |
| mhu | and therefore you also want a redirect URI per tenant | 17:04 |
| mhu | well, in all honesty, I know so little about react-redux and OIDC implementations for it that I might have got it all wrong | 17:05 |
| mhu | but "It works with my setup" (c) | 17:05 |
| corvus | mhu: and this is is the url for retrieving the oidc config right? https://zuul-ci.org/docs/zuul/discussion/components.html#attr-auth%20%3Cauthenticator%20name%3E.issuer_id | 17:08 |
| mhu | corvus, correct, see https://zuul-ci.org/docs/zuul/discussion/components.html#openidconnect | 17:09 |
| mhu | tomorrow I can write a howto somewhere about setting up keycloak with zuul, if you'd like to test stuff | 17:10 |
| corvus | mhu: what do you think about expanding the docs to include an example of setting up auth with a public provider (eg github or google)? | 17:10 |
| mhu | corvus, or that :) | 17:10 |
| corvus | heh -- they keycloak howto sounds like a good idea too, but i was thinking that using a public provider might make it easier for folks to test | 17:11 |
| mhu | I'm using keycloak for my tests, and keycloak then federates with github | 17:11 |
| corvus | you're doing the thing we want to do with opendev. :) cc infra-root ^ | 17:11 |
| mhu | but I can have a look into how to plug yourself directly with gh | 17:11 |
| fungi | neat! | 17:12 |
| corvus | mhu: if you have a spare moment, maybe you could look at https://review.opendev.org/731838 ? | 17:12 |
| mhu | corvus, well, more than happy to share that hard-earned knowledge, I only ask for reviews in exchange :) | 17:12 |
| fungi | and yeah, we're trying to describe our plans in that ^ though we have a bunch of productive discussion during the ptg last week which is not yet captured in the spec (see review comments for details) | 17:13 |
| corvus | mhu: you're getting reviews right now :) | 17:13 |
| mhu | corvus, thanks! I'd like the CORS_OPTIONS topic merged first if possible, because I'll have to rebase the rest on it | 17:14 |
| corvus | mhu: so yeah, i think at this point some kind of howto (at least one of: github, google, keycloak) would be very helpful in reviewing that, because i think that's probably the only way we're going to be able to test it, so we should get other zuul devs bootstrapped into an env where we can do that. | 17:14 |
| mhu | I mean I *could* just rebase the reviews but I'm starting to have a fairly long patch chain and I tend to mess up my cherry picks :D | 17:15 |
| corvus | mhu: i'll leave a comment on that change, but from what i can see so far, i think your approach looks good | 17:15 |
| *** hashar has joined #zuul | 17:16 | |
| corvus | mhu: i'll put those at the top of my list :) | 17:17 |
| *** sugaar has quit IRC | 17:32 | |
| *** reiterative has quit IRC | 17:32 | |
| *** reiterative has joined #zuul | 17:34 | |
| *** reiterative has quit IRC | 17:34 | |
| *** reiterative has joined #zuul | 17:35 | |
| *** sshnaidm|bbl is now known as sshnaidm | 17:40 | |
| *** hashar has quit IRC | 17:57 | |
| corvus | tobiash: fyi i'm working on the docker release tag thing | 18:04 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 18:11 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Run upload-docker-image on release https://review.opendev.org/734902 | 18:25 |
| corvus | tobiash, mordred, clarkb, fungi: ^ i think that should implement the simple version of what we discussed yesterday. | 18:26 |
| corvus | mnaser: ^ i think you were interested in that too | 18:26 |
| corvus | oh wait there's an error on that; sorry i changed approaches and forgot to correct | 18:27 |
| *** y2kenny has joined #zuul | 18:28 | |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Run upload-docker-image on release https://review.opendev.org/734902 | 18:31 |
| corvus | there we go | 18:31 |
| y2kenny | for nodepool, how do you folks decide between a provider vs pool vs labels? For example, is there any advantages to have multiple pool from the same provider vs having the pool sitting different provider of the same type? | 18:32 |
| corvus | tobiash, clarkb: if you could look at https://review.opendev.org/734902 soon i'd appreciate it -- we may need a couple of iterations through the pipeline to get that right; i'll be happy to do that after it merges | 18:33 |
| *** dennis_effa has quit IRC | 18:33 | |
| corvus | y2kenny: it's usually based on whether you need to break up the quota in order to enable certain configurations | 18:33 |
| corvus | y2kenny: for example, if you have different networks, and you need some labels to provide nodes on network A, and some on network B, those may be 2 pools in a provider | 18:34 |
| y2kenny | ok | 18:35 |
| corvus | y2kenny: another example might be setting the quota differently. see the 2 pools for this provider: https://opendev.org/openstack/project-config/src/branch/master/nodepool/nl02.openstack.org.yaml#L167 | 18:35 |
| corvus | there's a larger main pool, and a smaller pool of larger nodes | 18:36 |
| *** bhavikdbavishi has quit IRC | 18:36 | |
| y2kenny | corvus: ok. I am trying decide where to put some configuration for the driver I am writing. | 18:37 |
| *** bhavikdbavishi has joined #zuul | 18:37 | |
| corvus | y2kenny: i'm not sure there's a hard and fast rule. sometimes it makes sense on a ProviderLabel; sometimes it makes sense in a ProviderPool. some of them we've put on both. | 18:39 |
| corvus | i think networks is an example of one that's on both | 18:40 |
| y2kenny | ok | 18:40 |
| *** bhavikdbavishi has quit IRC | 18:42 | |
| y2kenny | corvus: I am a bit unclear on the responsibility of a nodepool driver. As far as I can tell, it gets the resources ready (with launches) but is it also responsible to record which resources are in use and quota? | 18:43 |
| y2kenny | or is the resource tracking done by nodepool itself? | 18:43 |
| y2kenny | I am looking at both the static driver and k8s/openshift driver and things seems to behave a bit differently. | 18:44 |
| corvus | y2kenny: it's a fuzzy cooperative arrangement and it's different for different drivers. that's something i've started trying to address with the 'simple' driver interface (that the gce driver is built on) | 18:44 |
| y2kenny | may be because k8s's is more 'soft' whereas static driver is more 'hard'? | 18:45 |
| corvus | y2kenny: the static driver is the most 'special' because it's internal; i'd avoid using it for a reference unless necessary | 18:45 |
| y2kenny | corvus:... um... but it is actually closest in terms of characteristics (I am working on the cobbler/baremetal driver right now) | 18:46 |
| y2kenny | I looked at the gce driver as well but there are a lot of stuff that doesn't quite match for baremetal provisioning (like having a cloud-image, for example.) | 18:47 |
| fungi | interesting that cobbler's bare metal provisioning doesn't normally use boot images. ironic's does | 18:48 |
| y2kenny | my current thinking is kind of a mix of k8s namespace type and static driver... essentially nodepool provision the baremetal node like a k8s namespace (in the sense that nodepool doesn't do much with it) | 18:48 |
| y2kenny | fungi: cobbler actually has concept of profile and such but I am not really using that | 18:49 |
| corvus | y2kenny: in all drivers, nodepool shouldn't do much with anything :) | 18:50 |
| fungi | ironic basically boots a small ramdisk and then retrieves and provisions the server's filesystem using that, before rebooting into it, so more cloud-like in that regard i guess | 18:50 |
| corvus | y2kenny: the general approach is to interface to the cloud system. so openstack requests a vm, k8s requests a pod or namespace. and i'd expect cobbler to request a baremetal machine. | 18:50 |
| y2kenny | fungi: or... I am actually using it in a more dynamic fashion. For my use case, I want to boot the system with a fairly dynamic image. Like an image that contains an experimental kernel that zuul just build so I wouldn't be able to provision that image via nodepool config | 18:50 |
| y2kenny | crovus: yes, I am making it so that the cobbler driver essentially return the ipmi address of the baremetal machine | 18:51 |
| y2kenny | corvus: what I am trying to understand right now is how (or if) I need to register the machines with nodepool so that the provisioning is correct. | 18:52 |
| corvus | y2kenny: cobbler maintains its own inventory, right? | 18:52 |
| y2kenny | corvus: for k8s namespace, it's almost like there's no registration of the 'resource' with nodepool | 18:52 |
| y2kenny | corvus: yes on the inventory bit | 18:52 |
| fungi | i would expect cobbler to tell nodepool about machines, not have nodepool track them independently | 18:52 |
| corvus | k i think i know what you're asking; 1 sec | 18:53 |
| y2kenny | fungi: but nodepool will have to track which machine is currently in use right? | 18:53 |
| fungi | yeah, i'm probably misunderstanding | 18:53 |
| y2kenny | looking at static drivier, looks like I need to 'register' the machine with a zk.Node() | 18:53 |
| fungi | i would expect the nodepool driver to tell cobbler to boot something matching a set of parameters, and then cobbler to return the relevant connection information to the nodepool driver | 18:54 |
| corvus | y2kenny: for this part, try to keep to the k8s driver for reference -- this is where the static driver is going to behave too differently. | 18:54 |
| y2kenny | whereas k8s-namespace I just launch | 18:54 |
| corvus | y2kenny: look at the K8SLauncher class | 18:55 |
| y2kenny | corvus: ok | 18:55 |
| y2kenny | corvus: ok but that's kind of what I meant in my confusion | 18:55 |
| y2kenny | corvus: K8SLauncher pretty much just create the soft resources on the fly | 18:55 |
| y2kenny | I am also looking at listNodes for the k8s provider | 18:56 |
| corvus | y2kenny: i would imagine that's where you'd ask cobbler for a machine, then set the connection information on the 'node' object | 18:57 |
| y2kenny | corvus: I think I need to rephrase my question... something is calling launch and that something is nodepool right? | 18:59 |
| corvus | y2kenny: yes | 18:59 |
| corvus | the nodepool launcher, specifically | 19:00 |
| y2kenny | corvus: how does nodepool descide which node to call launch(self, node) with? | 19:00 |
| corvus | y2kenny: it creates a new node for the request | 19:00 |
| y2kenny | um... | 19:00 |
| corvus | a node in nodepool is ephemeral | 19:01 |
| fungi | from a logistics standpoint the driver should just ask cobbler "give me an available machine, i don't care which one" or maybe "give me an available machine with 16 cpu cores" but the driver shouldn't need to keep track of which machines are available to cobbler, cobbler should pick something suitable from its inventory and tell nodepool what to use | 19:01 |
| y2kenny | fungi: oooh... so it's up to cobbler to track if a server is in use or not? | 19:02 |
| corvus | yes | 19:02 |
| fungi | nodepool tracks whether nodes are in use, it doesn't need to know about nodes which are available though, just maybe how many are available so it can stop asking if there are none | 19:02 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 19:03 |
| y2kenny | ok... but this is why I am looking at static driver as well... because for static nodes, nodepool track which is in use right? | 19:03 |
| fungi | basically nodepool wants to have a record of what nodes have been allocated which are in use by zuul, and then wants to tell the provider when it's done with a node so the provider can clean it up and add it back to the available pool | 19:04 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 19:04 |
| avass | so I've been spending a couple of hours trying to figure out why my tests for instance profiles didn't work.... | 19:04 |
| avass | it looks like that's just not supported by moto... https://github.com/spulec/moto/blob/master/moto/ec2/models.py#L474 | 19:04 |
| fungi | as corvus said, the static driver is an anomaly, it's basically a workaround for "i'm going to manage my own nodes outside the nodepool launcher" | 19:04 |
| avass | :( | 19:04 |
| *** rlandy is now known as rlandy|mtg | 19:05 | |
| corvus | the static driver is a one-off exception. we do some really convoluted stuff in the static driver to map it into nodepool's worldview. generally nodepool is an interface to a cloud system (doesn't have to be virtual -- baremetal clouds are cool) | 19:05 |
| *** sshnaidm is now known as sshnaidm|afk | 19:05 | |
| avass | unless I'm missing something patchset 9 of https://review.opendev.org/#/c/734774/ should work otherwise | 19:05 |
| corvus | but we don't want to turn nodepool into its own cloud system -- we've seen how hard of a job that is to do :) | 19:06 |
| avass | if anyone has time, can you take a look at 734774, should be an easy enough for review | 19:07 |
| corvus | i have to run to lunch, biab | 19:07 |
| y2kenny | fungi, corvus: um... then I probably need to rethink this a bit. Because until now, I basically see cobbler just a provider of some inventory and it doesn't track usage. | 19:11 |
| fungi | y2kenny: to summarize, if cobbler doesn't provide an api with functions like "tell me how many systems are available matching this set of parameters," "provision an available system matching this set of parameters and tell me how to connect to it," and "i'm done with this system you can clean it up now and make it available again" then there will probably need to be some service which sits between the nodepool | 19:17 |
| fungi | provider driver and cobbler to provide that abstraction layer | 19:18 |
| fungi | that's basically what bare metal cloud services like ironic are designed to provide though | 19:18 |
| y2kenny | fungi: I think my use case is perhaps a bit too weird. The level of provisioning I need in this case is really minimal, to the level of the k8s-namespace type. All I want to make sure is the same node not being use by two different jobs. Baremetal cloud is a bit overkill because I don't even want something else to load the base os image for me | 19:38 |
| y2kenny | (essentially just give me a baremetal system powered off and I will do the rest.) | 19:38 |
| y2kenny | this is because launching of the base os is part of the CI job that can potentially fail. | 19:39 |
| y2kenny | (custom kernel under test and all that.) | 19:42 |
| fungi | got it, so your jobs are talking ipmi (among other protocols) to these nodes | 19:47 |
| y2kenny | fungi: exactly | 19:47 |
| y2kenny | fungi: what I am hoping to get nodepool to really just to return the control interface to the machine and not necessary the machine itself | 19:49 |
| fungi | in that case i guess you need a service which provides the ipmi interface and other relevant network plumbing information from your "cloud" layer to the nodepool provider driver, though that service would ideally also take care of whatever cleanup steps are required (reflashing firmware/bios, wiping disks, et cetera) when nodepool tells it that the node is no longer in use | 19:49 |
| y2kenny | fungi: pretty much yes. Although the clean up can be part of Zuul job's cleanup-run as well. | 19:52 |
| fungi | so the provider driver in nodepool would ask that service for relevant connection information for the server, that service would allocate it and return the info, then nodepool would aggregate it into whatever set is necessary to meet zuul's node request and pass along the information for all the fulfilling nodes. later when zuul finished with the node, nodepool would tell the intermediary "cloud" service it was | 19:53 |
| fungi | done with the node at which point nodepool would forget about it and the cloud would clean up the node and make it available for future requests | 19:53 |
| fungi | remember zuul can't guarantee cleanup will succeed (disconnects and other sorts of interruptions could leak those nodes back in a dirty state) | 19:54 |
| fungi | zuul cleanup playbooks are best-effort | 19:54 |
| y2kenny | fungi: ok, right. | 19:55 |
| fungi | if that intermediate service can also report on availability counts (essentially "quota" available/used numbers) then the launcher can also refrain from requesting additional nodes when it knows are none available (or not enough to satisfy a given request) | 19:57 |
| y2kenny | fungi: is this 'quota' thing communicated to nodepool via listNode? | 19:58 |
| y2kenny | listNodes() | 19:58 |
| fungi | that's part of how it tries to work out available quota, yes | 19:58 |
| fungi | in part because some clouds have a bit of a delay updating used counts in their api responses | 19:59 |
| fungi | so it can get into a bit of a hysteresis around the full mark | 19:59 |
| y2kenny | I am actually not too clear on the schema of the Node to be return by listNodes(). For k8s, it seems to return FakeServer | 20:00 |
| y2kenny | um... I should take a look at gce driver. Right now I am assuming the labels are part of the node return by listNodes but at this point I haven't found the relevant code for it. | 20:01 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 20:21 |
| *** rlandy|mtg is now known as rlandy | 20:23 | |
| clarkb | corvus: why does the tagging change remove the pypi and docs updates? | 20:32 |
| corvus | clarkb: see last pgraph of commit msg | 20:32 |
| corvus | (which we want to do because it's important that we have version tags so ppl can pin to 3) | 20:34 |
| corvus | (and we have cut our last expected 3.x release already) | 20:34 |
| clarkb | corvus: ++ I +2'd it with a note for a potential followup when those jobs are reenabled | 20:35 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 20:37 |
| avass | fixed the pep8 on that^, if anyone has a good idea on how to test that better without too much effort I'm open t suggestions :) | 20:38 |
| openstackgerrit | Albin Vass proposed zuul/nodepool master: aws: add support for attaching instance profiles https://review.opendev.org/734774 | 20:38 |
| openstackgerrit | Merged zuul/zuul-jobs master: Allow upload-docker-image role to be used outside of promote https://review.opendev.org/734890 | 20:55 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Add descriptions to the different dashboard jobs https://review.opendev.org/729773 | 21:17 |
| *** harrymichal has quit IRC | 21:17 | |
| *** rfolco|rover has quit IRC | 21:23 | |
| openstackgerrit | Merged zuul/zuul master: Replace deprecated Thread.isAlive() with Thread.is_alive() https://review.opendev.org/732157 | 21:36 |
| *** threestrands has joined #zuul | 21:55 | |
| openstackgerrit | Merged zuul/zuul master: executor: Fix 'merge_jobs' configuration https://review.opendev.org/733958 | 22:03 |
| *** armstrongs has joined #zuul | 22:05 | |
| openstackgerrit | Merged zuul/zuul master: GitHub Reporter: Fix `Reviewed-by` in Merge Commit Message https://review.opendev.org/734580 | 22:10 |
| openstackgerrit | Merged zuul/zuul master: Update access token url https://review.opendev.org/734793 | 22:13 |
| *** armstrongs has quit IRC | 22:13 | |
| openstackgerrit | Merged zuul/zuul master: zuul_unreachable: Fix ansible callback exception https://review.opendev.org/734797 | 22:14 |
| openstackgerrit | Merged zuul/zuul master: Don't recreate parse context for every config file https://review.opendev.org/728774 | 22:22 |
| openstackgerrit | Merged zuul/zuul master: gerrit: disable poller when no http authentication is defined https://review.opendev.org/730655 | 22:22 |
| openstackgerrit | Merged zuul/zuul master: Add descriptions to the different dashboard jobs https://review.opendev.org/729773 | 22:22 |
| *** tosky has quit IRC | 23:15 | |
| *** rlandy has quit IRC | 23:39 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!