| anastasiamac | wallyworld: +1 | 00:06 |
|---|---|---|
| wallyworld | hpidcock: or kelvinli_: for today sometime, no rush https://github.com/juju/juju/pull/10848 | 00:58 |
| hpidcock | np | 00:59 |
| kelvinli_ | wallyworld: so we wanna make it support LB? I thought this change was just for microk8s | 01:01 |
| wallyworld | it's for k8s in general to handle the case where our opinionated default service type isn't suitable, eg we say loadbalancer but they just want clusterip, or visa versa | 01:03 |
| wallyworld | and to support externalName | 01:03 |
| wallyworld | this is just for controller service at the moment | 01:03 |
| wallyworld | to plug the gap where just cannot bootstrap due to different k8s cluster set ups | 01:04 |
| kelvinli_ | ic | 01:04 |
| wallyworld | the initial use case was microk8s to add in externalName support | 01:04 |
| wallyworld | but it's applicable everywhere really | 01:05 |
| wallyworld | ty for review | 01:05 |
| wallyworld | hpidcock: for 2.7.1, this adds the proxy-update-worker to the caas operator. it's already there for the k8s controller https://github.com/juju/juju/pull/10855 | 03:21 |
| hpidcock | alright | 03:21 |
| hpidcock | wallyworld: sorry got distracted, so the proxyupdater worker writes out conf files with the proxies so that hooks can use them correct? | 04:49 |
| wallyworld | hpidcock: that's only on iaas. for k8s (and also iaas) it unsures that http clients constructed have the proxy values set correctly | 04:50 |
| wallyworld | *ensures | 04:50 |
| wallyworld | we already run that worker in the k8s and iaas controller | 04:50 |
| wallyworld | and unit agent | 04:50 |
| wallyworld | but not k8s operator | 04:50 |
| wallyworld | juju automatically adds the controller ip address to no-proxy | 04:51 |
| wallyworld | to augment anything else that the user sets. so having that worker ensures that the operator agent can always see the controller | 04:52 |
| hpidcock | hmm did you test exec with this? | 04:52 |
| wallyworld | not exec. is it failing for you? | 04:54 |
| hpidcock | no I'm just trying to think of things that the operator does that isn't talking to the controller | 04:55 |
| wallyworld | i can't think of a pathway where it would fail | 04:55 |
| hpidcock | we might need to add the k8s controller ips to the no-proxy | 04:55 |
| wallyworld | juju does that automatically | 04:55 |
| hpidcock | 10/10 | 04:55 |
| wallyworld | it adds them to juju-no-prpxy | 04:55 |
| hpidcock | I mean, kube api | 04:55 |
| hpidcock | not juju controller | 04:56 |
| wallyworld | i see that as a cluster setup thing? | 04:56 |
| wallyworld | outside of juju | 04:56 |
| wallyworld | the juju-proxy settings are just to make juju agents work | 04:56 |
| wallyworld | and charms | 04:56 |
| wallyworld | the charm can see the juju-proxy values | 04:57 |
| wallyworld | and can use them as it needs | 04:57 |
| wallyworld | that's one of the reasons why we deviated from the standard proxy values | 04:57 |
| hpidcock | yeah but `settings.SetEnvironmentValues()` sets env vars on the agent | 04:57 |
| wallyworld | yes | 04:57 |
| wallyworld | and we expose those JUJU_ values to the charm hook context | 04:58 |
| hpidcock | hmm well the vendored github.com/juju/proxy uses "HTTPS_PROXY" etc | 04:59 |
| wallyworld | we use different values on purpose | 04:59 |
| wallyworld | so as not to interfere with any system values and let the charms decide how they want to use them | 05:00 |
| wallyworld | we kept getting bit by corner cases | 05:00 |
| wallyworld | and decided to make the juju configured proxy values totally separate | 05:00 |
| wallyworld | and juju stuff (controller, agents, charms) would use those values | 05:01 |
| hpidcock | ok so we need to document that if you specify proxy settings you may need to add your k8s apiserver address to no_proxy | 05:08 |
| hpidcock | do you know if that is specified anywhere? | 05:08 |
| wallyworld | hmmm, in the cloud metadata endpoint? | 05:08 |
| wallyworld | it would be the same issue for aws or vm clouds i think | 05:09 |
| wallyworld | i don't think k8s api endpoint would be any different in that regard | 05:09 |
| wallyworld | i am not sure what our doc says off hand | 05:10 |
| hpidcock | ok fair enough | 05:11 |
| hpidcock | thanks for clarifying | 05:11 |
| wallyworld | there is some doc https://jaas.ai/docs/offline-mode-strategies | 05:12 |
| wallyworld | https://jaas.ai/docs/configuring-offline-usage | 05:12 |
| wallyworld | which seems to cover what to put in no-proxy | 05:12 |
| wallyworld | but maybe doesn't call it out explictly | 05:13 |
| anastasiamac | wallyworld: tiny PR PTAL :D https://github.com/juju/juju/pull/10858 | 07:37 |
| wallyworld | ok | 07:38 |
| wallyworld | anastasiamac: i don't get why we don;t think controller clouds have credentials. they must have credentials as they are in use | 07:42 |
| wallyworld | if i add-k8s and use that cluster, there's a credental there | 07:42 |
| anastasiamac | wallyworld: i don't think we get this info from api... | 07:42 |
| anastasiamac | when we list clouds | 07:43 |
| anastasiamac | irrespective... is it really relevant for controller clouds?.. | 07:43 |
| wallyworld | just trying to understand the root cause | 07:43 |
| anastasiamac | we'd want to list all of them, no? | 07:43 |
| wallyworld | for controller i'd think so | 07:43 |
| wallyworld | if the user has perm to see them | 07:44 |
| anastasiamac | wallyworld: so whether controller clouds have creds or not, does not mater - we want them :D | 07:44 |
| anastasiamac | so this check is redundant and there is no need to change api.. | 07:44 |
| wallyworld | sgtm | 07:45 |
| anastasiamac | \o/ | 07:46 |
| nammn_de | manadart: can you take a quick look at this one? https://github.com/juju/juju/pull/10833 | 10:29 |
| nammn_de | we can change something for review :P | 10:29 |
| manadart | nammn_de: Looks good. 2 suggestions. | 11:01 |
| nammn_de | stickupkid achilleasa you guys worked with it before. Want to give a really small review? https://github.com/juju/charmrepo/pull/156 Description has reason | 11:06 |
| achilleasa | nammn_de: looking | 11:07 |
| stickupkid | nammn_de, do you want to wire it up in juju and change the dep package source | 11:07 |
| stickupkid | ? | 11:07 |
| nammn_de | stickupkid: was planning to do so in the near future. Either I change from juju/juju how we create the repo and charmarchive or I add something to charmrepo | 11:08 |
| nammn_de | achilleasa manadart: ta | 11:08 |
| stickupkid | nammn_de, i'd get everything full end to end, so you don't see churn? | 11:09 |
| nammn_de | stickupkid: dependenciy wise changing the place to tmp for charm testing is actually quite the hassle, I must say :/ | 11:09 |
| achilleasa | nammn_de: where are we going to use this? | 11:09 |
| nammn_de | achilleasa: I was planning to migrate some code here: https://github.com/juju/juju/pull/10836/files | 11:11 |
| nammn_de | reason: currently our charm tests are taken from the juju/juju path as a repo and used from there. Thus we create a version string from there and the tests fail because they expect us to parse the version file. Solution we discussed: Moving them to a tmp path | 11:11 |
| nammn_de | As it seems a lot of tests are using the `repo` from `charmrepo`. But as it currently is, I cannot migrate the code, because it forces me to use a relative path | 11:12 |
| achilleasa | nammn_de: fair enough; thanks for the info | 11:12 |
| nammn_de | achilleasa: thanks 🦹‍♂️ | 11:13 |
| nammn_de | woah got the feeling that i have to touch 100 places (and repos) for one small change meeeeeh xD | 11:14 |
| nammn_de | stickupkid: before adding the repo I tried a workaround to see whether it would work here: https://github.com/juju/juju/pull/10836/files A better way would be to change the testsuite and create a tmp repo there and access it from the tests. But imo this would be more related to an additional pr. With the new change in repo I would "just" change | 11:22 |
| nammn_de | this to be a "tmpRepo" https://github.com/juju/juju/blob/dfd597542ef537162a0ecff27e6a4095c23adc1e/testcharms/charm.go#L42 instead of "tmpArchive" Wdyt? | 11:22 |
| nammn_de | stickupkid: for now I only changed the places where the tests failed (2 places) to use a tmpcharm instead of our repo | 11:23 |
| stickupkid | nammn_de, i'd go with the lightest touch atm | 11:23 |
| nammn_de | stickupkid: great, one the same page here | 11:23 |
| nammn_de | stickupkid: thats the final change i suggest, if test go through. https://github.com/juju/juju/pull/10836 | 11:57 |
| stickupkid | nammn_de, yeah, lets wait for all the tests to go through, but looks much better | 11:58 |
| === fenris is now known as Guest81494 | ||
| verterok | bdx: hi! found an issue with memcache interface, my proposed fix is: https://github.com/omnivector-solutions/interface-memcache/pull/2 | 13:54 |
| nammn_de | manadart: I tried to incoporate your suggestions https://github.com/juju/juju/pull/10833 | 14:08 |
| achilleasa | stickupkid: any particular reason why the root test runner is not using !/bin/bash? | 14:16 |
| stickupkid | because portability | 14:16 |
| stickupkid | achilleasa, i've said previously if people want to move to bash then we can do, but I think we don't really need to | 14:16 |
| achilleasa | no 'set -e pipefail' on my box :-( | 14:17 |
| stickupkid | yeah, that's bitten me | 14:17 |
| stickupkid | THAT is the ONLY thing I want | 14:17 |
| stickupkid | the work around is to split :( | 14:17 |
| stickupkid | but on the plus side it reduces complexity of a one liner | 14:18 |
| achilleasa | stickupkid: so how can I make a helper func abort the test with an error message? | 14:20 |
| stickupkid | achilleasa, lets try it, we can always improve | 14:20 |
| hml | stickupkid: what’s the expectation for which directory the integrations tests are run from? | 14:21 |
| stickupkid | hml, features should have their own, but we should have a group of tests that we expect to be groupable | 14:22 |
| hml | stickupkid: i’m having “fun” with specifying the path to a local bundle yaml | 14:23 |
| stickupkid | hml, deploy = stuff around deploy, machines = add machines etc, cli = test failures, help messages | 14:23 |
| hml | stickupkid: right | 14:23 |
| hml | stickupkid: if you run main.sh blah blah blah.. .from which directory? juju? tests? | 14:23 |
| stickupkid | tests | 14:23 |
| stickupkid | tests is the root directory, everything changes into there and runs main.sh | 14:24 |
| hml | stickupkid: rgr got confused by “bundle” path in run_deploy_cmr_bundle() | 14:24 |
| hml | stickupkid: but i can change it | 14:25 |
| manadart | hml: Mind taking a look at https://github.com/juju/juju/pull/10859 ? | 15:00 |
| hml | manadart: sure | 15:00 |
| manadart | hml: Ta. | 15:00 |
| hml | stickupkid: review pls? https://github.com/juju/juju/pull/10860 I made some changes to the cmr bundle test in there. Hopefully i wasn’t just running things wrong. :-D | 15:00 |
| stickupkid | hml, looks spot on, let me test it | 15:03 |
| hml | manadart: do we have an O7k cloud with multiple zones to test with? | 15:10 |
| manadart | hml: Don't know. Both serverstack and canonistack all list just "nova". | 15:10 |
| stickupkid | hml, a most, it's just paths. | 15:21 |
| stickupkid | all* | 15:21 |
| hml | stickupkid: yeah, just saw the lint failures | 15:21 |
| stickupkid | hml, once I applied the fixes, it worked as designed :D | 15:22 |
| hml | stickupkid: ho? | 15:22 |
| stickupkid | hml, of course | 15:22 |
| hml | manadart: reviewed with 2 small things. | 16:17 |
| manadart | hml: Thanks. The Cinder tests do verify the AZ at https://github.com/juju/juju/pull/10859/files#diff-2dc3b00e7504a446604d294e712eae0aR180 | 16:19 |
| hml | manadart: ah, missed that. ty | 16:20 |
| hml | stickupkid: updated 10860 | 16:58 |
| stickupkid | hml, https://github.com/juju/juju/pull/10862 | 17:52 |
| hml | stickupkid: looking | 17:53 |
| hml | stickupkid: hey… though ./main.sh was the wrong way to run those tests. :-D | 17:58 |
| nammn_de | if someone has time for a really small line? https://github.com/juju/charm/pull/298 Old pr forgot to remove old logger | 20:55 |
| hml | nammn_de: timClicks got there before me. :-) | 20:57 |
| nammn_de | ha, thanks timClicks hml was just trying to get things running under the testsuite and saw weird logging messages :-D | 20:58 |
| timClicks | nammn_de: shouldn't you shut down your computer? | 20:59 |
| timClicks | it's late there | 20:59 |
| nammn_de | timClicks: haha dont mind me, im leaving. Have a nice day ! | 21:04 |
| rick_h | night nammn_de | 21:07 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!