| maccer | hello! I'm looking for some pointers/instructions to install cloud-init on a macbook, would anyone here be able to help? | 09:21 |
|---|---|---|
| === hjensas|afk is now known as hjensas | ||
| lucasmoura | blackboxsw, I have pushed my description solution on the apt schema branch. If you want to discuss it, just let me know | 13:24 |
| Odd_Bloke | So I did a little more poking at that pytest issue, and I think the issue is that our CiTestCase monkey-patching of util.subp causes pylint to be unable to correctly infer the type of util.subp (because it sees those CiTestCase assignments and knows that it can't infer what's going on there). It's still not clear to me why the results have been inconsistent, but my best guess is that there is some | 13:59 |
| Odd_Bloke | difference between the processes that means some of them do find the CiTestCase assignments (and so can't infer the call signature of util.subp, and so don't emit a message about us calling it incorrectly) and some don't (and therefore correctly infer the signature of util.subp, and so do emit the message). | 13:59 |
| Odd_Bloke | I may have inadvertently "fixed" this in master by adding the tests in https://github.com/canonical/cloud-init/pull/343; that imports CiTestCase in test_conftest, so I would now expect all pylint workers to find the CiTestCase assignments (and therefore _never_ infer util.subp's type correctly and therefore never report the error). | 14:01 |
| Odd_Bloke | blackboxsw: +1 on the ubuntu/devel PR! \o/ | 16:50 |
| blackboxsw | wooooooot! | 16:51 |
| * blackboxsw is digging out from reviews at the moment. | 16:51 | |
| Odd_Bloke | Dang, they left, I'm very curious to know what that person was expecting cloud-init to do on a Macbook. :p | 16:52 |
| blackboxsw | "take over the world", just like the rest of us | 16:53 |
| blackboxsw | Odd_Bloke: wasn't there another person trying to get configy type things going on a mac cloud a couple weeks ago? | 16:54 |
| blackboxsw | or maybe I inferred that from their question | 16:54 |
| meena | has anyone made an attempt at making net/ platform independent?er… | 18:37 |
| meena | i'm looking at the code again. | 18:38 |
| Odd_Bloke | I just noticed a gap in our fail-if-subp-is-called coverage on our tests: we configure it for pytest in cloudinit/conftest.py, which means it applies to everything under cloudinit/. However, we have also introduced pytest-based tests under tests/. I can see two paths forward: move conftest.py up a directory (to the repo root, so it affects all tests in the repo), or relocate the existing pytest-based | 18:42 |
| Odd_Bloke | tests (there are 4 classes, so this isn't much work) under cloudinit/ and update our unit test documentation to prohibit introducing pytest-based tests under tests/*. What do people think? | 18:42 |
| meena | robjo: heyo, wanna rewrite cloudinit/net? | 19:10 |
| robjo | ENOTIME | 19:11 |
| meena | anyone else? :P | 19:18 |
| meena | where did we land on? do we transform cloudinit/net into a class or a function dispatcher? | 19:19 |
| blackboxsw | lucasmoura: sorry for the delay, your changes to schema.py look really good/appropriate. Couple minor nits on https://github.com/canonical/cloud-init/pull/357#pullrequestreview-411120453 and we are good. At some point we'll have to have a discussion about how to move to draft7 of jsonschema (as xenial/bionic/other distros don't support that). But that's a conversation for another time . | 19:39 |
| Odd_Bloke | blackboxsw: I see you've moved the ubuntu/devel card to Daily in our board, but I don't see an upload and the PR is still open. Did you move it over prematurely, perhaps? | 19:43 |
| Odd_Bloke | blackboxsw: And, unrelatedly, do you know of any compelling reasons for us to move to draft7 of JSON Schema? (Not asking you to do any investigation, just curious if you happen to know of any off-hand.) | 19:44 |
| blackboxsw | blackboxsw: yep. premature card move turns out you have to hit <enter> to upload :) | 19:48 |
| * blackboxsw kicks that now-ish. it'll be ready by daily tomorrow | 19:49 | |
| Odd_Bloke | ^_^ | 19:49 |
| blackboxsw | Odd_Bloke: yes patternProperties | 19:49 |
| lucasmoura | blackboxsw, No problem and I have fixed the typos you have commented | 19:49 |
| blackboxsw | as it stands any cloud-config module which has opaque keys like cc_apt_configure "sources" is not represented in schema | 19:49 |
| blackboxsw | the schema in draft4 is permissive and just says, make sure we are a dict | 19:50 |
| blackboxsw | it can't describe the sub-objects of each sources item | 19:50 |
| blackboxsw | draft7 lets us describe that like this: | 19:53 |
| blackboxsw | https://paste.ubuntu.com/p/Yf3tGrtJ6R/ | 19:54 |
| blackboxsw | without draft7 we basically have to be very permissive on nested objects below opaque keys | 19:55 |
| blackboxsw | so as it stands, our existing draft4 schema won't error on invalid apt:\n sources:\n 'nogood' | 19:56 |
| Odd_Bloke | Interesting, thanks! | 19:56 |
| blackboxsw | so same kindof question applies to curtin | 19:57 |
| blackboxsw | and storage config validation | 19:57 |
| blackboxsw | but that's another ball of work/big feature for a cycle | 19:57 |
| blackboxsw | hrm Odd_Bloke lucasmoura hold the phone. I may have mapped that wrong | 19:59 |
| blackboxsw | /usr/lib/python3/dist-packages/jsonschema/schemas/draft4.json in your focal/groovy Ubuntu distro shows patternProperties there | 20:00 |
| blackboxsw | in draft4. so, hrm we might be able to get that on cc_apt_configure. I'm testing now. which would mean we could use this for lucas's current branch. But, let's leave that for a followup PR | 20:01 |
| Odd_Bloke | blackboxsw: It's not clear to me that we need patternProperties for this case, actually: aren't the patterns for the key names (which are well-defined)? | 20:02 |
| Odd_Bloke | *well-defined in this case | 20:02 |
| Odd_Bloke | (Either way, agreed we shouldn't block landing the in-flight PR.) | 20:02 |
| blackboxsw | Odd_Bloke: not for sources: https://cloudinit.readthedocs.io/en/latest/topics/examples.html#additional-apt-configuration-and-repositories | 20:03 |
| Odd_Bloke | Oh riiight, yeah. | 20:03 |
| blackboxsw | each top-level key under apt:sources: <any-string>: {keyid: keyserver:... source} | 20:03 |
| blackboxsw | yeah. again it looks like a was mistaken about patternProperties being in draft7. If so, I think we can clear this up for draft4. Also I think there may be 2-3 other config modules that don't define 'strict' schema for certain config objects living below the scope of opaque strings. | 20:05 |
| blackboxsw | I'll add a card and we can talk about this item tomorrow/later | 20:05 |
| amansi26 | rharper: A small clarification needed. When we deploy a VM. Interface is getting assigned by cloud init, but once the VM get deployed, if we manually got and assigned some other interface. Is cloud init comes in pic there also. | 20:31 |
| amansi26 | according to my understanding, it basically takes the details from config Drive(provided by Nova) and configure the interfaces. Correct me if I am wrong. | 20:32 |
| blackboxsw | lucasmoura: what would you prefer you squash merge commit message be for #357. Is this ok ? https://paste.ubuntu.com/p/NVRm9bnnqX/ | 20:40 |
| blackboxsw | I tried to trim a bit for the commit msg and went with line formatting < 80 chars | 20:41 |
| * blackboxsw should have wrapped at 72 chars | 20:41 | |
| lucasmoura | blackboxsw, I am okay with the commit message you proposed | 20:41 |
| blackboxsw | done merged thx lucasmoura | 20:45 |
| meena | let's test how to this fares on Hetzner: https://github.com/canonical/cloud-init/pull/363 | 20:46 |
| amansi26 | blackboxsw: Can you please clearify my above doubt? | 20:46 |
| Odd_Bloke | blackboxsw: I think we may be able to do what we want with additionalProperties and items; patternProperties would only be necessary if we had different item schemas depending on what the key is. (I'll push up one half of the snap change for your review before I do the other half, so you can point out whatever I'm missing. ;) | 20:47 |
| blackboxsw | amansi26: sorry, context switching. ok cloud-init comes into the picture rendering initial network config on most datasources whenever a new instance-id is detected from the datasource metadata. in DataSourceConfigDrive case. I'm trying to refresh myself about network config behavior | 20:51 |
| blackboxsw | so amansi26 https://github.com/canonical/cloud-init/blob/master/cloudinit/sources/DataSourceConfigDrive.py#L142 is what tells us if the system is considered new, and if new, network would be re-rendered for whatever new from openstack's network_data.json | 20:52 |
| amansi26 | So in case we add a new network, it will create a new instance id? | 20:54 |
| blackboxsw | or rather the 'networkdata' or 'network_config' on the configdrive | 20:54 |
| amansi26 | but the config drive come in picture at the time of initial deployment. After that if we add any network, this configDrive will not get updated with the new interface details | 20:55 |
| amansi26 | blackboxsw: then how come the system will get to know a new network has to be added. Is it done through cloud init | 20:58 |
| blackboxsw | amansi26: cloud-init generally is a boot time configuration, some datasource platforms support re-rendering network config across each boot (because metadata is expected to change providing new network information). I'm trying to wrap my head around how best to do this with configdrive | 20:59 |
| blackboxsw | I'm trying to find you a solution that doesn't involve a hammer :) | 21:00 |
| blackboxsw | if your configdrive metadata isn't being changed to update the config drive's metadata(with instance-id set) or networkdata (with openstack's networkdata.json format) or network_config (which is etc/network/interfaces fomat file content). then I don't know how cloud-init would 'known' about the new interfaces | 21:03 |
| === tds1 is now known as tds | ||
| blackboxsw | for what it's work amansi26 rharper is working toward a hotplug solution that watches udev device adds to a vm | 21:04 |
| blackboxsw | for what it's worth*.... | 21:04 |
| blackboxsw | that would react to seeing a new device plugged into your vm after the fact, but that would only trigger a re-crawl of metadata (ConfigDrive or openstack's network_data.json) to react to the full network config intended for that device) | 21:05 |
| blackboxsw | so configdrive front for the moment, I believe your environment would have to surface a new system-uuid value at /sys/class/dmi/system-uuid, which tells cloud-init to re-run. Your config drive content in networkdata or network_config would have to represent the newly added device | 21:10 |
| blackboxsw | and cloud-init would pick that up | 21:11 |
| Odd_Bloke | blackboxsw: OK, so patternProperties actually causes us problems. Currently we treat keys as opaque, which means if they are accidentally integers (instead of strings as they would have to be in JSON), we don't error now, but patternProperties tracebacks (not emits a warning, or fails to validate; tracebacks) if keys aren't strings. | 21:11 |
| Odd_Bloke | (Perhaps this is what you discovered before when evaluating it.) | 21:12 |
| blackboxsw | hrm that could be Odd_Bloke sorry for the waste there if that's the case. It's been a while since I tried exercising it. At least that would confirm, can't do this on draft4. so a noop for the moment. My only reasong to reject my previous sentiment about moving to draft7 was that I saw the patternProperties definition today in the draft4.json schema definition and assumed functionality was there. | 21:14 |
| Odd_Bloke | blackboxsw: The functionality is there, but relies on keys being strings (which they _have to be_ in JSON, but do not have to be in YAML or Python dicts), so we can't use it trivially. | 21:17 |
| blackboxsw | interesting, so patternProperties requires strings, we can't provide a '\d+' reference? | 21:17 |
| Odd_Bloke | We could, but that would match "123456" not 123456. | 21:17 |
| blackboxsw | ahh right right. | 21:17 |
| blackboxsw | hrm | 21:17 |
| blackboxsw | I hadn't gotten to that as far as implementation | 21:17 |
| Odd_Bloke | Regardless, I still don't think we actually need it for our cases. | 21:18 |
| blackboxsw | so Odd_Bloke to clarify: 1. you think we can find a way to describe sub-objects under opaque keys, or 2. you think we can continue to ignore strict validation of sub-objects under opaque keys | 21:19 |
| Odd_Bloke | blackboxsw: https://github.com/canonical/cloud-init/pull/364 <-- I think we can describe sub-objects under opaque keys | 21:19 |
| Odd_Bloke | Oh wait, I'm missing a commit there, I got distracted by testing patternProperties and didn't finish writing the commit message. :p | 21:20 |
| Odd_Bloke | blackboxsw: OK, pushed. | 21:21 |
| blackboxsw | Odd_Bloke: yeah commands didn't help us in this case. | 21:21 |
| blackboxsw | fetching again | 21:22 |
| blackboxsw | Odd_Bloke: but with assertions as dict, we don't have a definition of the properties in that dict anywhere | 21:24 |
| blackboxsw | assertions as object rather | 21:24 |
| blackboxsw | items': {'type': 'string'} applies only to list types | 21:24 |
| blackboxsw | it doesn't vet object types | 21:24 |
| Odd_Bloke | blackboxsw: I only modified commands, I haven't touched assertions yet. | 21:25 |
| blackboxsw | hrm... .ok sorry,drawing up the counter example for commands too | 21:26 |
| blackboxsw | Odd_Bloke: https://paste.ubuntu.com/p/FkKCdRvPwN/ should fail your schema python3 -m cloudinit.cmd.main devel schema -c this.yaml | 21:28 |
| blackboxsw | but doesn't | 21:28 |
| Odd_Bloke | That's still using assertions. :p | 21:28 |
| blackboxsw | hah, oops | 21:29 |
| blackboxsw | bummer. sooo yeah. maybe I'm wrong ... for the first time in my life...... in the last hour | 21:30 |
| Odd_Bloke | Folks, we got him. | 21:30 |
| Odd_Bloke | blackboxsw: Yeah, I think if you're only specifying one of them, `additionalProperties: ...` is basically the same as `patternProperties: { ".*": ...}`; both match all keys that aren't matched by `properties`. | 21:31 |
| blackboxsw | ok excellent Odd_Bloke yes I was using the silly wrong key. | 21:31 |
| blackboxsw | awesome. TIL | 21:31 |
| blackboxsw | ok that'll do. good deal. so that'll work for cc_apt_configure apt:sources too I think. | 21:32 |
| Odd_Bloke | And thinking about it, I would expect needing patternProperties is an indication that our config format is wonky; we shouldn't be switching behaviour based on substrings in configuration keys, they should all be absolute. | 21:33 |
| blackboxsw | Odd_Bloke: with your branch, I think there is also some fallout due to simplitic failure annotation handling too. `python3 -m cloudinit.cmd.main devel schema -c snap.yaml --annotate` traces | 21:33 |
| blackboxsw | *simplistic error annotation tracking* | 21:33 |
| Odd_Bloke | blackboxsw: Hmm, OK, sounds like we perhaps have a testing gap because all the tests pass. Let me fill that gap then fix the failure(s). :) | 21:34 |
| blackboxsw | Odd_Bloke: yes, I think annotations testing is very minimal | 21:34 |
| blackboxsw | and since we don't have a set of bad examples in different config formats we only have a couple of cases in AnnotatedCloudconfigFileTest tests/unittests/test_handler/test_schema.py | 21:35 |
| blackboxsw | so different object type failures and not covered at all. | 21:36 |
| meena | hrm… cloud-init isn't recognizing that i'm on Hetzner… once again… | 21:41 |
| meena | other than that, my code seems to have worked… | 21:46 |
| meena | updated with comments on what works and what doesn't | 21:49 |
| meena | https://github.com/canonical/cloud-init/pull/363 that is. | 21:49 |
| meena | now, bed / sleep, etc… | 21:49 |
| meena | how do i override vendor data again? | 21:52 |
| Odd_Bloke | blackboxsw: OK, so I think the error you're seeing is because you're using integer keys. | 21:54 |
| Odd_Bloke | (Which people will do, so we do need to handle it.) | 21:54 |
| blackboxsw | Odd_Bloke: strange github not allowing me to update branch in the UI now https://github.com/canonical/cloud-init/pull/347 | 21:55 |
| blackboxsw | approved btw | 21:55 |
| Odd_Bloke | blackboxsw: https://paste.ubuntu.com/p/MmXtpXjjrZ/ <-- it's fine with string keys, not with integer keys | 21:56 |
| blackboxsw | ahh nice. ok limited impact. but still noise. annotate is a half-baked feature that will break as we extend our schema use/validation tests. :/ | 21:57 |
| blackboxsw | community notice: Just uploaded cloud-init 20.2+ to groovy gorilla (20.10) [ubuntu/groovy-proposed] cloud-init 20.2-20-gd10ce3ec-0ubuntu1 | 21:58 |
| meena | https://travis-ci.org/github/canonical/cloud-init/jobs/686766728 ← something's wrong with the fonts here, or my eyes | 21:59 |
| meena | given the tme, i'll say eyes. | 21:59 |
| Odd_Bloke | meena: No I think that text is meant to be red. ;) | 21:59 |
| blackboxsw | heh. | 22:00 |
| Odd_Bloke | blackboxsw: Actually it's even more specific than that: the problem is using integer keys which aren't string-represented by Python the same as the way they are expressed. So 00 is a problem because `"00" != str(int("00"))`. | 22:00 |
| Odd_Bloke | But 0 is fine, for example. | 22:01 |
| blackboxsw | hah | 22:02 |
| blackboxsw | right | 22:02 |
| Odd_Bloke | And I'm not sure there's a good way to address that, unfortunately. | 22:07 |
| Odd_Bloke | Because ints are valid keys in YAML, when we read the YAML we lose the original string representation. So I don't know how we would avoid it. | 22:08 |
| Odd_Bloke | Probably the best bet is to (a) make sure we aren't using keys that would fail in any examples, and (b) document that annotation will break if you use integer keys. | 22:08 |
| Odd_Bloke | (And (c) that patches to address that are welcome. ;) | 22:08 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!