| thumper | looking for another review of https://github.com/juju/juju/pull/11208 | 01:03 |
|---|---|---|
| hpidcock | thumper: I had a quick glance over it, but might be better for wallyworld to review it more thoroughly | 01:24 |
| wallyworld | looking | 01:26 |
| wallyworld | thumper: to what extent were you still intending to look at john's unresolved comments? the only other issue is the one hpidcock raised. were you lokking for a +1 or waiting for john? | 02:00 |
| wallyworld | hpidcock: here's that show-operation one https://github.com/juju/juju/pull/11238 | 02:01 |
| thumper | wallyworld: john's main unresolved issue was the initialization case, where we were calculating the summary hash for every model change during initialization | 02:01 |
| thumper | this was the addition of the initializing variable and hooking into mark and sweep | 02:01 |
| thumper | I was looking for a +1 | 02:02 |
| thumper | I'll update the controller's mark and sweep to lock | 02:02 |
| wallyworld | thumper: there's also hr passing of the address of initialising variable | 02:02 |
| wallyworld | that could open t up to a race condition | 02:03 |
| thumper | not really | 02:03 |
| thumper | or at least the race detector doesn't think so | 02:03 |
| thumper | there are memory barriers around the access and udpates | 02:04 |
| wallyworld | ok | 02:04 |
| wallyworld | thumper: you have your +1 | 02:05 |
| * thumper goes to make updates for hpidcock | 02:05 | |
| hpidcock | wallyworld: I added comments on your PR | 04:14 |
| wallyworld | ta | 04:14 |
| wallyworld | kelvinliu_: your pr fixes bug 1854104 right? | 04:21 |
| mup | Bug #1854104: Duplicate file mounts in CaaS charm causes silent failure <k8s> <juju:In Progress by kelvin.liu> <https://launchpad.net/bugs/1854104> | 04:21 |
| kelvinliu_ | wallyworld: yes | 04:21 |
| wallyworld | ta, i'll add link to pr | 04:21 |
| kelvinliu_ | we were editing the description at the same time, but u committed quicker. lol | 04:24 |
| hpidcock | wallyworld: how about this for a kicker, aws returns a `Your requested instance type (c4.large) is not supported in your requested Availability Zone (ap-northeast-1a)` even though the instance type is supported according to their APIs. And to top it off, they also use that error when there isn't any stock available. | 04:29 |
| hpidcock | so nothing we can do about that atm | 04:30 |
| hpidcock | I've fixed the cases where the AZ straight up doesn't support the instance type | 04:30 |
| wallyworld | hpidcock: well how about that, jeez | 04:38 |
| wallyworld | kelvinliu_: got time for HO? | 04:38 |
| kelvinliu_ | yep | 04:38 |
| hpidcock | wallyworld: also apne1-az3 (which is our ap-northeast-1a) looks to be either a small AZ or they are decommissioning it. So this could be the source of the problems | 04:39 |
| wallyworld | kelvinliu_: google hangouts sound not working, will try reboot | 04:41 |
| kelvinliu_ | nws | 04:42 |
| wallyworld | kelvinliu_: ah wait, i think it works again | 04:42 |
| wallyworld | kelvinliu_: am in HO | 04:43 |
| kelvinliu_ | me 2 | 04:44 |
| kelvinliu_ | ru in standup? | 04:44 |
| hpidcock | wallyworld: I believe I've found the bug. When we retry a StartInstance call, we don't retry on failed AZs. But, there was logic to check if there are no AZs left to try on, and to clear the failed flag for that machine. But we only checked to see if there were ANY AZs that we could retry on. So if the constraints filtered out at least one AZ, we would never clear the failed flags. | 05:38 |
| hpidcock | So when filtering to one AZ, it would only ever try once to start the instance. | 05:39 |
| wallyworld | hpidcock: interesting, i could have sworn we had a special error type to deal with az failures | 05:42 |
| anastasiamac | hpidcock: also for the instance types that are no longer suported by cloud providers, they can be marked as deprecated.. i believe that will elminiate them from the running... | 05:48 |
| hpidcock | anastasiamac: problem is this specific AZ, AWS APIs report them as being supported. And they are not deprecated yet (still considered current generation according to AWS) | 05:50 |
| anastasiamac | ah | 05:50 |
| wallyworld | hpidcock: ty for review also, i cargo culted that "interesting" timer code from actions. i'll refactor all the instances | 05:50 |
| wallyworld | hpidcock: so is the plan to rework the retry logic on AZ failure? | 05:51 |
| hpidcock | no it's a tiny change, the retry logic just needed to handle constraints | 05:52 |
| hpidcock | it handles constraints when selecting an az, not when retrying the AZs the constraints match | 05:52 |
| hpidcock | I'll throw a PR up soon | 05:53 |
| wallyworld | gr8 ok | 05:53 |
| hpidcock | can probably wait till monday https://github.com/juju/juju/pull/11239 happy to change anything | 06:17 |
| wallyworld | hpidcock: looking | 06:18 |
| wallyworld | hpidcock: should github.com/hpidcock/aws-fetch-instance-types live alongside process_cost_data.go in ec2/internal/instancetypes | 06:20 |
| hpidcock | wallyworld: I didn't want to introduce aws-sdk | 06:21 |
| hpidcock | but I'm happy to | 06:21 |
| wallyworld | ah i see | 06:22 |
| wallyworld | we have the k8s api, azure sdk etc | 06:22 |
| wallyworld | one more may not hurt | 06:23 |
| wallyworld | lol "17,114 additions, 3,649 deletions not shown because the diff is too large. Please use a local Git client to view these changes." | 06:24 |
| hpidcock | just a few changes | 06:25 |
| wallyworld | hpidcock: btw, the reason the wait duration was a string was to allow the user to leave off the duration and have it default to seconds | 06:27 |
| wallyworld | i don't like it but it's a compatibility change :-( could stick it behnd a the feature flag i guess | 06:28 |
| wallyworld | all code that's 5 years old | 06:28 |
| hpidcock | --wait 1 | 06:28 |
| hpidcock | 1 what lol | 06:28 |
| wallyworld | yeah, i know, i know | 06:29 |
| wallyworld | it will be abit messy but i can do show-operation the right way and hack up the legacy show-action-output to use the feature flag | 06:30 |
| wallyworld | since show-task and show-action-output are the same command | 06:31 |
| wallyworld | but with a different name for jujuv3 feature flag vs legacy | 06:31 |
| wallyworld | and show-operation was copied across from show-action-output | 06:31 |
| === parlos_afk is now known as parlos | ||
| === parlos is now known as parlos_afk | ||
| === parlos_afk is now known as parlos | ||
| matt_kosut | icey: hi I was proposing a small improvement on ansible apply_playbook in charm-helpers about how arguments are passed. Wondering if it seems worth dicussing https://github.com/juju/charm-helpers/pull/425 | 08:56 |
| icey | matt_kosut: I've never even looked at the ansible bits of charmhelpers :-P | 09:03 |
| matt_kosut | icey: github kinda suggested you as reviewer, should I look for someone else in there? :D | 09:04 |
| icey | matt_kosut: one of the existing contributors, maybe? I can review the code but I don't have any context on the change | 09:06 |
| matt_kosut | tinwood: back to you again :-) you have been the last one reviewing ansible part of charm-helpers past few years, maybe you could have more context to have a look on my PR? https://github.com/juju/charm-helpers/pull/425 | 09:10 |
| zeestrat | ^ and a couple more Ansible PRs: https://github.com/juju/charm-helpers/pull/403, https://github.com/juju/charm-helpers/pull/404 | 09:55 |
| tinwood | matt_kosut, zeestrat, I'm not really an ansible person ... i.e. I don't whether these things are 'correct' or not. However, I'm happy to merge if you can review each other's patches. | 10:15 |
| manadart | stickupkid, achilleasa: Need a review of https://github.com/juju/juju/pull/11241 | 10:22 |
| matt_kosut | tinwood: sounds reasonable too, I used it quite lot past few weeks :) | 10:23 |
| zeestrat | @tinwood I understand. Just for the record, matt_kosut and me are colleagues so are slightly biased, but I have looked over his and it looks good. I suggested adding a docstring to clarify, should be good to go after that. | 10:33 |
| stickupkid | manadart, looking | 10:41 |
| stickupkid | manadart, done | 11:06 |
| manadart | stickupkid: Ta. | 11:06 |
| stickupkid | manadart, the test code for goose doesn't obviously fill in the device id :| | 11:09 |
| stickupkid | manadart, makes it difficult to test | 11:10 |
| manadart | ●_● | 11:13 |
| zeestrat | tinwood: Things are reviewed and ready to go when you are. | 11:13 |
| tinwood | zeestrat, matt_kosut please could you bring those branches up to date with the current head of master; I can do it, but then you'd need to pull the changes if you wanted to make any more mods to the branches (e.g. if the merge broke a test for example). | 12:34 |
| zeestrat | tinwood: should be up to date now. | 12:48 |
| matt_kosut | tinwood: same on my side, good to go. :-) | 12:50 |
| tinwood | zeestrat, matt_kosut just working my way through them; they take a while as each time one is merged, the branch needs to be brought up to date and then the tests have to run ... I'll get back to you! :) | 12:55 |
| zeestrat | tinwood: ty! I fixed conflict in https://github.com/juju/charm-helpers/pull/403. | 12:56 |
| tinwood | zeestrat, kk - I'll keep an eye on them. I've just merged https://github.com/juju/charm-helpers/pull/425 so it may happen again? | 12:58 |
| zeestrat | tinwood: Looks good now. | 13:11 |
| tinwood | zeestrat, matt_kosut that should be all three merged. Thanks for your work on them! | 13:36 |
| zeestrat | tinwood: nice. Thank you very much and have a nice weekend :) | 13:40 |
| tinwood | you too! | 13:40 |
| matt_kosut | thanks! | 13:42 |
| hml | achilleasa: approved 11237 | 14:20 |
| achilleasa | hml: tyvm | 14:21 |
| stickupkid | manadart, https://github.com/go-goose/goose/pull/79 | 14:57 |
| manadart | stickupkid: Approved it. | 15:13 |
| stickupkid | ta much | 15:15 |
| stickupkid | manadart https://github.com/juju/juju/pull/11235 | 15:37 |
| manadart | stickupkid: Swap you. https://github.com/juju/juju/pull/11243 | 15:44 |
| skay | how do I configure a file via the command line? | 15:46 |
| skay | I mean, a config setting | 15:46 |
| skay | my yaml file uses include-file | 15:47 |
| skay | a ha! got help from a friend. juju config app value=@/path/to/file | 15:49 |
| hml | region pr review: https://github.com/juju/juju/pull/11242 | 15:54 |
| stickupkid | manadart, done | 16:04 |
| manadart | stickupkid: Ta. | 16:06 |
| hml | stickupkid: ty | 16:37 |
| achilleasa | hml: or stickupkid any recommendations for playing with windows workloads? | 16:48 |
| hml | achilleasa: never done it. :-D | 16:48 |
| achilleasa | hml: hmmm... maybe windows vm and manual? | 16:49 |
| achilleasa | I want to try the reboot check | 16:49 |
| hml | achilleasa: there’s a noop charm in the store? | 16:49 |
| hml | achilleasa: https://jaas.ai/u/ionutbalutoiu/noop/7 | 16:49 |
| hml | but might not have enough for your purposes | 16:50 |
| achilleasa | all a need is a unit agent | 16:50 |
| achilleasa | s/a/I/ | 16:50 |
| hml | that should get ya one | 16:50 |
| achilleasa | awesome thanks | 16:50 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!