| === shardy is now known as shardy_afk | ||
| === shardy_afk is now known as shardy | ||
| Be-El | hi | 11:11 |
|---|---|---|
| Be-El | we are planning to provide a shared filesystem in our cloud, and the information how to mount is should be stored in the vendor data. but we want users to have more control over it, e.g. enable/disable mounting | 11:14 |
| Be-El | is there a simple way to do this, or does it require an extension to cloud init itself? | 11:14 |
| === asthasr_ is now known as asthasr | ||
| smoser | Be-El, who is "we" are you the cloud provider or the user? and what is the cloud ? | 13:27 |
| smoser | if cloud-init gets storage configuration that says to mount things, it will write those things into /etc/fstab. | 13:28 |
| smoser | user-data can be used to override any vendor-data provided. | 13:28 |
| Be-El | 'we' are a group of several cloud providers working together and trying to provide a similar service to our users | 13:28 |
| Be-El | the details for each site (e.g. filesystem, host names / ip address of fileservers) may and will vary | 13:29 |
| Be-El | the cloud instances are all based on openstack, and using the package and mount modules works so far | 13:34 |
| smoser | so if the user wants to override the data you've provided they can provide 'mounts: []' | 13:35 |
| smoser | that should work, then they'll get none. user-data wins over vendor-data. | 13:35 |
| Be-El | but I want to restrict the functionality to certain images (afaik there's no image metadata that can be accessed by cloudinit) and allow the user to disable it | 13:35 |
| smoser | so you want the vendor-data to only appear to certain images ? is that what you're saying ? | 13:36 |
| Be-El | that would be my preferred solution | 13:37 |
| smoser | well, when i originally added vendor-data support on openstack i did so in a way that you could implement a python class on the host. | 13:37 |
| smoser | that class was given the instance-id and told to give vendor-data | 13:37 |
| smoser | the default class just read from the /etc/.../vendor_data.json | 13:38 |
| smoser | (this is from memory) | 13:38 |
| smoser | but then at some point openstack started ripping out things like that. so that extensible functionality might be gone now | 13:38 |
| Be-El | I'm currently thinking about a helper script whose execution is triggered by runcmd in vendor-data | 13:40 |
| Be-El | and the script checks for the existence of some touched file (-> user data) and skips the mount operation | 13:40 |
| Be-El | script arguments will define the mount options etc. | 13:40 |
| Be-El | doable, simple design, but feels wrong | 13:41 |
| smoser | well, if you want to provide different vendor-data to different instances based on some criteria, that was originally supported. | 13:42 |
| smoser | and i think that is what "would be [your] preferred solution". | 13:43 |
| smoser | so honestly that is what i would go for. | 13:45 |
| smoser | it looks like current openstack trunk has some mechanism to run a vendor-data service and have nova client plug into that. | 13:46 |
| smoser | i'm just looking at git logs | 13:46 |
| Be-El | the clouds are all based on the newton release, and openstack version upgrades are not that easy... ;-) | 13:46 |
| dpb1 | heh | 13:47 |
| Be-El | there's a vendordata_driver option, but it is marked as deprecated | 13:47 |
| smoser | well. 1f53bfcc7998f63f130a2cedaf15b41a4506c568 | 13:47 |
| smoser | is a good commit message | 13:47 |
| smoser | https://review.openstack.org/gitweb?p=openstack/nova.git;a=commitdiff;h=1f53bfcc7998f63f130a2cedaf15b41a4506c568 | 13:48 |
| smoser | "It is intended that this fix be backported to newton as well." | 13:48 |
| Be-El | newton also has a mechanism for using external rest services instead of a simple file | 13:50 |
| Be-El | not as easy as just providing the python class, but i'll have a closer look at it. thx for the hints | 13:51 |
| Be-El | does cloud-init already use vendor-data.json and vendor-data2.json? | 13:55 |
| smoser | does not read vendor-data2.json only vendor-data.json | 13:56 |
| smoser | what is interesting to me is that they coudl have used the existing class functionality to implement the remote rest api stuff | 13:56 |
| smoser | and thus kept backward compatibility a | 13:56 |
| smoser | oh well | 13:56 |
| smoser | but i guess they wanted to be more strict on the extensibility portion | 13:57 |
| Be-El | and backwards compatibility, data merging.... | 14:04 |
| smoser | blackboxsw, rharper i'm working on bug 1715128 | 14:07 |
| ubot5` | bug 1715128 in cloud-init (Ubuntu) "Crashes in convert_ec2_metadata_network_config on ScalingStack bos01 (ppc64el)" [Critical,Confirmed] https://launchpad.net/bugs/1715128 | 14:07 |
| smoser | Be-El, are you poking fun at me! | 14:07 |
| smoser | (its entirely acceptable) | 14:07 |
| Be-El | i'm german, we don't know how to have fun | 14:08 |
| rharper | smoser: yeah; two things 1) we certainly should check that we have 'network' in the metadata dictionary safely 2) EC2Local runs before OpenStack datasource; that's generally a problem if local datasources are positively identified | 14:13 |
| rharper | for Azure, we confirm UUID; and I thought that EC2 in Artful was in strict mode, which should have forced the UUID check, no ? | 14:13 |
| smoser | rharper, well, yeah. | 14:17 |
| rharper | oh, right | 14:17 |
| rharper | well, even on non-intel, the system has a UUID, so unless they rolled unluckily and got ec2* for their UUID | 14:17 |
| smoser | https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330361 | 14:24 |
| smoser | rharper, systems only have uuid (from dmi) on intel | 14:25 |
| smoser | not sure what you were meaning | 14:25 |
| rharper | on intel, yes via dmi; in ppc64 though, they do set a uuid value from the hypervisor; they just don't send any product or other DMI related values (and it's not under the dmi path in sys) | 14:35 |
| rharper | that info won't identify it as OpenStack, but it can be used to check that it's not EC2 (which I think is the path you're taking w.r.t EC2Local); | 14:36 |
| rharper | as other platforms run in Local mode and hit network; I think we need to require the positive ID to run the local variant | 14:36 |
| smoser | well.. due to clones it can't identify "not ec2" | 14:40 |
| smoser | we could be more strict yes. but we are not, so that it generally still works but give the warning, so we can fix things later (on non-intel). | 14:41 |
| smoser | on intel, cloud-init would have disabled completely | 14:41 |
| rharper | do the clones report ec2 in UUID ? I didn't think any of them actually did that vs. just subclassing EC2 and providing something else to identify themselves | 14:44 |
| smoser | unknown clones is the concern. | 14:49 |
| smoser | as they do not identify themselves at all and rely upon the old default behavior of falling back to ec2 | 14:49 |
| rharper | ec2 in network mode though | 14:50 |
| smoser | if you said "uuid doesnt start with ec2, so exit", then they'd get no warning | 14:50 |
| powersj | smoser: rharper: can one of you respond to https://bugs.launchpad.net/cloud-init/+bug/1711963 Reporter is asking how to get vendor and user data working together. | 15:29 |
| ubot5` | Ubuntu bug 1711963 in cloud-init "unable to merge vendor-data and user-data" [Undecided,Incomplete] | 15:29 |
| smoser | this is wierd | 15:36 |
| smoser | 2017-09-07 06:03:25,838 - stages.py[INFO]: Skipping modules ['runcmd'] because they are not verified on distro 'ubuntu'. To run anyway, add them to 'unverified_modules' in config. | 15:36 |
| rharper | whoa | 15:39 |
| rharper | smoser: where's that from ? | 15:39 |
| smoser | that was in the log on https://bugs.launchpad.net/ubuntu/+source/cloud-init/+bug/1715128 | 15:45 |
| ubot5` | Ubuntu bug 1715128 in cloud-init (Ubuntu) "Crashes in convert_ec2_metadata_network_config on ScalingStack bos01 (ppc64el)" [Critical,Confirmed] | 15:45 |
| rharper | powersj: on that one maybe smoser; by my read there isn't anyway to "merge" keys from vendor data into user-data; it's an override situation only ; ie, if you set the same key in user-data; that's all you get; smoser do you expect that we could support a case where user-data wants to inspect/inherit vendor-data values ? | 15:48 |
| powersj | rharper: thx for looking - that's what I thought and tried to say in my response, so hopefully having someone else convey it | 15:49 |
| smoser | well it merges in some way. | 15:51 |
| rharper | powersj: I think you did; the submitter is right that "merging" config isn't clear about which sorts of configs get merged (and which dont) | 15:51 |
| smoser | it certainly *should* use the full merge support | 15:51 |
| rharper | I think the request is to merge conflicting keys between user-data and vendor-data; which I've always understand that to not be mergable w.r.t combining keys | 15:52 |
| rharper | that is, how would a user override vendor-data if it's always merged together ? | 15:52 |
| smoser | user-data always "wins". | 15:53 |
| rharper | right | 15:54 |
| rharper | in this case, I think the submitter wants to modify that | 15:54 |
| smoser | rharper, what is wierd in 'Skipping' above is that later in log | 15:54 |
| smoser | 2017-09-07 03:06:10,312 - helpers.py[DEBUG]: Running config-runcmd using lock (<FileLock using file '/var/lib/cloud/instances/06c0123d-0393-44c7-87f5-3fb4734ffdd2/sem/config_runcmd'>) | 15:54 |
| rharper | smoser: I can't find any record of that "unverified" string in cloud-init ; where is that coming from ? | 15:55 |
| rharper | smoser: this is a proposed migration, right? so maybe it's being upgraded and rebooted ? | 15:55 |
| rharper | so same instance but different behavior due to reboot and upgrade ? | 15:55 |
| smoser | cloudinit/stages.py | 15:56 |
| smoser | if skipped: | 15:56 |
| smoser | LOG.info("Skipping modules %s because they are not verified " | 15:56 |
| smoser | "on distro '%s'. To run anyway, add them to " | 15:56 |
| smoser | "'unverified_modules' in config.", skipped, d_name) | 15:56 |
| smoser | i really can't come up with anything on that other than if they've added a cc_runcmd somewhere. | 15:56 |
| smoser | i dont know | 15:56 |
| === shardy is now known as shardy_afk | ||
| rharper | commit cc9762a2d737ead386ffb9f067adc5e543224560 | 15:57 |
| rharper | Author: Chad Smith <chad.smith@canonical.com> | 15:57 |
| rharper | Date: Tue Aug 22 20:06:20 2017 -0600 | 15:57 |
| rharper | schema cli: Add schema subcommand to cloud-init cli and cc_runcmd schema | 15:57 |
| rharper | that added the 'distros' tag to the module; possible they're on older artful image, booted, upgraded cloud-init, rebooted | 15:57 |
| rharper | and we don't handle that ? | 15:57 |
| blackboxsw | reading backlog context now | 15:58 |
| rharper | smoser: AFAICT this is a separate issue (unrelated to ec2 local on non-intel) | 15:58 |
| rharper | we can file a separate bug for that I think | 15:58 |
| blackboxsw | whoa, so if I add a distros attr to the module it requires some level of validation | 16:00 |
| blackboxsw | ok looking at what we are doing there w/ the skip | 16:00 |
| blackboxsw | strange as the module documentation claimed "all" was supported | 16:00 |
| blackboxsw | (I think I also added a distros attr to cc_bootcmd too) | 16:01 |
| blackboxsw | in a recent branch | 16:01 |
| smoser | blackboxsw, yeah, i didn't understand how 'all' was working | 16:02 |
| smoser | but it sure seems to be. otherwise runcmd would log that error everywhere. | 16:02 |
| blackboxsw | yeah looks like we extrapolate mod.distros as worked_distros and check against the actual distro name. ok, so we need to expand 'all' to all known cloud init distros or define DISTROS_ALL = None | 16:07 |
| blackboxsw | just filed #1715690 I'll get a branch together and I'm adding a unit test to https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330361 | 16:15 |
| blackboxsw | https://bugs.launchpad.net/cloud-init/+bug/1715690 rather | 16:16 |
| ubot5` | Ubuntu bug 1715690 in cloud-init "Defining distros = 'all' in a module for documentation results in a module skip as 'all' doesn't match distro 'x'" [High,New] | 16:16 |
| smoser | ah. ok. so your chagne is new. | 16:21 |
| smoser | that is good. understood now. | 16:21 |
| blackboxsw | smoser: yeah and it affects the existing bootcmd branch too. | 16:21 |
| blackboxsw | too bad we do discovery on distro modules, I was thinking it would be nice to have a get_all_distro_names() method, but that'd be expensive as I need to find_modules and collect all the names | 16:22 |
| blackboxsw | maybe that's an okay expense as we do it only once in stages | 16:22 |
| smoser | blackboxsw, i *think* runcmd is straight up busted then, right? | 16:23 |
| smoser | as in an integration test of: | 16:23 |
| smoser | runcmd: | 16:23 |
| smoser | - [touch, /run/foobar] | 16:23 |
| smoser | would show that failure | 16:23 |
| smoser | right? | 16:23 |
| blackboxsw | yeah it'll skip it because it defines distros 'all' which doens't match actualy distro name | 16:23 |
| blackboxsw | yeah I'll add an integration test | 16:23 |
| smoser | blackboxsw, thanks. | 16:23 |
| smoser | blackboxsw, so... on my networkign thing in ec2local | 16:24 |
| smoser | i was saying i wanted to raise NotImplemented | 16:24 |
| blackboxsw | I like the branch, wanted a tiny unit test but yeah | 16:24 |
| smoser | but returning None is the same from the consumer's perspective | 16:24 |
| smoser | ie, if that property (its a @property) is None | 16:24 |
| smoser | then it is ignored | 16:24 |
| blackboxsw | true, you mean from network_config | 16:24 |
| blackboxsw | ? | 16:25 |
| smoser | so returning None on that (and logging HEY THIS IS BUSTED) | 16:25 |
| smoser | yes, network_config | 16:25 |
| smoser | so if we lgged fail on that and returned None (as it did not actually have it) then we'd be better off | 16:25 |
| smoser | but the issue with returning None is that it caches based on None | 16:25 |
| blackboxsw | yes so network_config property in base could be smarter about 'network' KeyError and return None there | 16:25 |
| smoser | hm.. let me show you what i was thinking | 16:26 |
| rharper | smoser: where is the network-config cache check ? | 16:26 |
| blackboxsw | was wondering about this http://pastebin.ubuntu.com/25484907/ | 16:27 |
| blackboxsw | rharper: line 287 of DataSourceEc2.py if self._network_config is None | 16:28 |
| rharper | certainly checking post convert if the value is still none would be valid | 16:29 |
| rharper | but I guess I'm not getting the smoser is suggesting; I think it's reasonable for the EC2 local ds to crawl metadata, not find the 'network' key and raise NotImplemented; in stages, the _find_network_config would need a try: except NotImplemented and can log the NotImplemetnted exception (which ec2 could report that the metadata didn't have the 'network' key needed) | 16:31 |
| rharper | s/the smoser/what smoser; | 16:31 |
| smoser | http://paste.ubuntu.com/25484930/ | 16:32 |
| smoser | blackboxsw, ^ | 16:32 |
| smoser | None versus _unset ... not sure how you really are to do that. | 16:32 |
| smoser | but still want to cache the None result as work was done. | 16:32 |
| blackboxsw | smoser: in landscape we did UNSET = objecT() | 16:35 |
| blackboxsw | smoser: in landscape we did UNSET = object() | 16:35 |
| blackboxsw | but yeah I get it | 16:36 |
| blackboxsw | smoser: per rharper's comment, returning a None here on when we have an invalid datasource doesn't really cause cloudinit/stages.py to balk on the datasource, it proceeds with fallback nic right? | 16:38 |
| rharper | stages only cares if you have the attribute and assumes the network-config attr is valid | 16:39 |
| rharper | I think we still need a way to tell stages that the network-config may not be valid | 16:39 |
| blackboxsw | right so it feels maybe like our best approach is for get_data() to return false then right? | 16:46 |
| blackboxsw | ocessingrso we don't even get into the network_config p | 16:46 |
| blackboxsw | so we don't even get into the network_config processing | 16:46 |
| rharper | well; whether or not we identify a datasource, and whether a datasource has network-config support are separate things that both need addressing | 16:48 |
| rharper | the latter has a bug w.r.t a datasource claiming it has network-config support (which may not be valid) and stages assuming the presence of the attribute as an indication that the configuration in the attr is valid; | 16:49 |
| blackboxsw | doesn't testing the return value None from datasource.network_config tell us whether the config is valid or not? | 16:52 |
| blackboxsw | in the case of None, we know there is no helpful valid content provided by the datasource so it can be ignored in that case | 16:53 |
| rharper | yes, you're right | 16:54 |
| rharper | I see that now in find_network_config | 16:54 |
| blackboxsw | yeah per even the default dscfg = ('ds', None) | 16:55 |
| rharper | that works nicely with the attempted consuming of network metadata in smoser paste | 16:56 |
| blackboxsw | to clarify for me: so if our ec2 local datasource is running in an openstack environment, get_data will still 'work' because it's a lookalike metadata service. but network_config would in this case not define network_config info. Won't the datasourceEc2Local still "match? | 16:57 |
| blackboxsw | it'll just use fallback nic in this case | 16:57 |
| rharper | which is what we'd want for look-a-likes which may not implement network metadata | 16:57 |
| smoser | blackboxsw, i'd seen object() before too | 17:00 |
| blackboxsw | I guess so, I wasn't sure if we'd want cloud-init to limp along using a sub-optimal datasource. | 17:00 |
| smoser | but i was afraid of that across un-pickling | 17:01 |
| blackboxsw | smoser: fair point. /me can't wait to remove pickling | 17:01 |
| blackboxsw | .... and break things in the process | 17:01 |
| smoser | it shouldl return None | 17:01 |
| smoser | network_config attr of None | 17:01 |
| smoser | means "this datasource has no network config" | 17:01 |
| smoser | which is what the base class has | 17:02 |
| blackboxsw | yeah I think rharper and I are on the same page w/ that | 17:02 |
| smoser | and is what Ec2 had before it started to have that | 17:02 |
| smoser | then stages does the right thing | 17:02 |
| smoser | right | 17:02 |
| smoser | yeah. ok. | 17:02 |
| smoser | blackboxsw, so i'll commit the _unset thing ? | 17:03 |
| smoser | unless you have a better idea | 17:03 |
| blackboxsw | so in a situation where Ec2 is run against openstack. get_data still 'works' and network_config returns None so LocalEc2 will still get detected or are you also leaving in the get_data check on Platform.AWS? | 17:03 |
| rharper | Ec2Local shouldn't run under non-intel openstack; https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330361 I think covers that; | 17:08 |
| smoser | blackboxsw, yes. still leaqving the local check for platform.aws | 17:09 |
| blackboxsw | smoser: +1 on adding your pastebin to the existing patch, I only think network_md should be mandatory param to the convert_ function. and the change to accept network_md will probably affect unit tests http://pastebin.ubuntu.com/25485096/ | 17:12 |
| smoser | blackboxsw, right. i'll change it to take mandatory | 17:18 |
| smoser | and i have updated the test. | 17:18 |
| smoser | blackboxsw, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330361 | 17:19 |
| smoser | updated | 17:19 |
| blackboxsw | grabbing | 17:19 |
| smoser | blackboxsw, i'm just going to grag stangatori's branch | 17:27 |
| smoser | sound fine ? just merge | 17:28 |
| blackboxsw | smoser: +1 on that | 17:28 |
| blackboxsw | we can sort long-term cards for consolidation of comon 'imc' logic into cloud-init's net functions in subsequent cycles | 17:29 |
| smoser | blackboxsw, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330242 | 17:30 |
| smoser | can you quick OK that ? i addressed your feedback. | 17:30 |
| smoser | approve and mark approved | 17:30 |
| blackboxsw | smoser: approved | 17:31 |
| smoser | rharper, https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/329811 | 17:36 |
| smoser | can you just mark 'approved' ? you 'Need Fixing'd and then approved in words, but not in changing your status | 17:36 |
| dpb1 | smoser: so... #1711963. legit bug? | 17:58 |
| smoser | at very least a bug on doc | 18:01 |
| dpb1 | smoser: ya, the doc could use an example | 18:01 |
| blackboxsw | smoser: comments added here https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330361 +1 with the unit test | 18:08 |
| blackboxsw | smoser: left it as approved assuming the changeset passes CI (I tested locally w/ tox -e py27( | 18:09 |
| smoser | blackboxsw, i'll grab your test. | 18:14 |
| smoser | thanks | 18:15 |
| smoser | blackboxsw, di dyou have a merge request that you wanted me to look at ? | 18:24 |
| smoser | for https://bugs.launchpad.net/cloud-init/+bug/1715690 i thought. | 18:25 |
| ubot5` | Ubuntu bug 1715690 in cloud-init "Defining distros = 'all' in a module for documentation results in a module skip as 'all' doesn't match distro 'x'" [High,New] | 18:25 |
| blackboxsw | smoser: will put one up. I was sidetracked fiddling w/ unittests | 18:36 |
| blackboxsw | I need about 20 mins | 18:37 |
| blackboxsw | smoser: WDYT about a get_all_distros method in distros/__init__.py which calls importer.find_modules and returns a list of all discovered distro names supported | 18:37 |
| blackboxsw | we'd only have to call that function once at line 823 of cloudinit.stages | 18:40 |
| blackboxsw | I'll draw up a sample diff | 18:41 |
| blackboxsw | w/out unit tests for feedback | 18:41 |
| rharper | smoser: done | 18:43 |
| blackboxsw | side note, found a logic bug in stages | 18:43 |
| blackboxsw | so for all modules we call distros.Distro.expand_osfamily | 18:43 |
| blackboxsw | and we call expand_osfamilies(mod.osfamilies) and that'd raise a ValueError on undefined osfamilies. But an environment might have actually provided a pluggable Distro, which isn't tracked by expand_osfamilies. So they wouldn't actually be able to make use of that module | 18:46 |
| blackboxsw | it's a corner case that impacts only non-std Distros which aren't already included in cloud-init/distros/OSFAMILIES | 18:47 |
| * blackboxsw really dislikes the magic of def fixup_module(mod, def_freq=PER_INSTANCE): | 18:49 | |
| smoser | backwards compatibility | 18:50 |
| smoser | and originally crappy smoser code | 18:50 |
| smoser | make for crappy code | 18:50 |
| blackboxsw | as it magically sets module attributes distros, osfamilies, frequency (due to the sideeffects & operations in stages.py) | 18:50 |
| blackboxsw | hahaa | 18:50 |
| blackboxsw | I'd much rather explicitly see osfamilies = [], distros = [], osfamilies = [] in all cc_*py . Then, at least it's more explicit in the module things that a module should care about defining | 18:52 |
| blackboxsw | but yeah, not sure how that computes w/ backward compatibility | 18:53 |
| blackboxsw | ok, for real, let me get a branch together. | 18:53 |
| smoser | blackboxsw, ... | 19:13 |
| smoser | if a: | 19:13 |
| smoser | print("this is fine") | 19:13 |
| smoser | elif b: | 19:13 |
| smoser | print('this is also fine") | 19:13 |
| smoser | else: | 19:13 |
| smoser | # i want to assert something here in a unit test... | 19:14 |
| smoser | what do i use .. ? | 19:14 |
| smoser | ah. maybe just raise assertionError | 19:14 |
| blackboxsw | or just assert(something) | 19:15 |
| blackboxsw | and it'll cause that assertion error to be raised in unit tests | 19:15 |
| blackboxsw | I'm not sure that's what you meant | 19:15 |
| smoser | well, i just want the test to FAIL | 19:15 |
| smoser | i think | 19:16 |
| smoser | raise AssertionError("neither a nor b was true.") | 19:16 |
| blackboxsw | I have seen that in unit tests before. it'll work | 19:18 |
| smoser | rharper, commented https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/330361 | 19:37 |
| smoser | please read. | 19:37 |
| smoser | basically.. .if we ds.network_config raises an exception, then cloud-init will generally fail. and such a failure will likely result in us seeing it. | 19:38 |
| smoser | i'm not entirelsy opposed to having convert_ec2_metadata_network_config try better to check and raise ValueError and then catching that one error and returning None and logging WARNING. | 19:39 |
| blackboxsw | smoser: rharper a lot of our modules document that they are supported on distros: all. Do we want module docs to represent the actual distro names, or would 'all' suffice | 19:43 |
| blackboxsw | now that I have a get_all_distros function we could make docs specify: ['ubuntu', 'gentoo', 'freebsd', 'debian', 'fedora', 'centos', 'sles', 'rhel', 'arch', 'opensuse'] | 19:43 |
| blackboxsw | or just leave it at "distros: all" which could account for 3rd party distro modules added in another env | 19:44 |
| blackboxsw | "it" being documentation on rtd | 19:44 |
| smoser | blackboxsw, i think 'all' should suffice | 19:50 |
| smoser | it means all distros supported by cloud-init | 19:50 |
| smoser | no ? | 19:50 |
| blackboxsw | yes on all distros supported by cloud-init. And agreed, the generic 'all' feels better and doesn't preclude 3rd party distro addons if there were any | 19:51 |
| blackboxsw | strange is we already have an integration test which exercises runcmd. | 19:52 |
| blackboxsw | thrying to find out why that's not failing | 19:52 |
| smoser | hm. | 19:52 |
| rharper | blackboxsw: alternatively can we match any module if supported is 'all' ? | 19:55 |
| rharper | that would avoid a list expansion | 19:55 |
| blackboxsw | rharper: smoser just pushed functional https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+ref/config-modules-allow-distros-all | 20:03 |
| blackboxsw | it shows what I was thinking, I'm adding unit tests now | 20:03 |
| blackboxsw | https://git.launchpad.net/~chad.smith/cloud-init/commit/?id=34297260006768be70904cecd2d66b2b5d97d2c4 | 20:04 |
| blackboxsw | yeah rharper that agrees with your suggestion | 20:04 |
| blackboxsw | kindof | 20:04 |
| blackboxsw | actually it doesn't. heh. it expands if 'all' is listed | 20:05 |
| blackboxsw | lemme see if we can avoid it | 20:05 |
| blackboxsw | hah, maybe I'm reading this wrong, that Skipped log is only a log, it looks like we still actually run the module | 20:07 |
| blackboxsw | stages.run_section doesn't do anything with the skipped modules it finds | 20:08 |
| blackboxsw | it still blindly calls _run_modules(mostly_mods) | 20:08 |
| blackboxsw | it doesn't pop the module off or anything | 20:08 |
| smoser | blackboxsw, that would explain integration test passing | 20:10 |
| smoser | and lowers the severity of this bug :) | 20:10 |
| blackboxsw | yep indeed | 20:10 |
| blackboxsw | :) | 20:10 |
| blackboxsw | I was searching the logs of that bug you referenced to see if I can find a successfully ran runcmd too | 20:10 |
| smoser | rharper, what are you looking for on convert_ec2_... | 20:10 |
| rharper | it return None if it's not equivalent to fallback (ie, we have a subnet) | 20:11 |
| rharper | we should either have a subnet with dhcp, or a subnet with a static ip | 20:11 |
| rharper | otherwise, it's a busted network configuration | 20:11 |
| blackboxsw | on the flip side of the "skipping " not really skipping, it also looks like forcing doesn't actually force the module to run either | 20:11 |
| rharper | blackboxsw: =/ | 20:11 |
| blackboxsw | running unverified_modules: %s is just a log and has no bearing on whether the module is run | 20:12 |
| smoser | http://paste.ubuntu.com/25485882/ <-- | 20:12 |
| smoser | that has better validity checking. | 20:12 |
| smoser | i'll get one more check | 20:13 |
| blackboxsw | man that's a lot of bulletproofing smoser | 20:13 |
| blackboxsw | feels like it's leaning toward overkill, and that we would actually run into that ValueError or TypeError issue if we try to use the object as a dict | 20:15 |
| blackboxsw | >>> a ='non-dict' | 20:15 |
| blackboxsw | >>> a['interfaces'] | 20:15 |
| blackboxsw | Traceback (most recent call last): | 20:15 |
| blackboxsw | File "<stdin>", line 1, in <module> | 20:15 |
| blackboxsw | TypeError: string indices must be integers, not str | 20:15 |
| blackboxsw | >>> | 20:15 |
| rharper | I'm much more worried about the values and keys in the dictionary | 20:15 |
| blackboxsw | why check isinstance dict everywhere and not just try to use the object and handle typeerrors instead | 20:15 |
| rharper | mac address may be mixed case, we might not have public-ipvX key; any case where we get back a network-config that doesn't apply dhcp or a static ip is going to result in uncaught badness; in which case it would be better to report busted ec2 network config and use fallback | 20:16 |
| rharper | note, this is for a ec2-clone path | 20:16 |
| blackboxsw | heh, validated the case from the Skipping runcmd logs, I see it also runniing that runcmd log (and why our integration tests still pass) | 20:18 |
| blackboxsw | 2017-09-07 06:03:26,068 - stages.py[DEBUG]: Running module runcmd (<module 'cloudinit.config.cc_runcmd' from '/usr/lib/python3/dist-packages/cloudinit/config/cc_runcmd.py'>) with frequency once-per-instance | 20:18 |
| blackboxsw | updating the bug | 20:18 |
| smoser | blackboxsw, well, the individual checking gets you better error messages. is the only reason for that. | 20:19 |
| smoser | and KeyError being more vague | 20:19 |
| smoser | rharper, i dont necessarily agree that "better to report busted ec2 network config and use fallback" | 20:20 |
| smoser | because "reporting" means no one ever sees a problem because you "fixed" it for them. | 20:20 |
| rharper | that's fair | 20:20 |
| smoser | where as failing badly means you see errors | 20:20 |
| rharper | but see the bugs about non-intel identifying themselves in openstack for why to log and move forward | 20:20 |
| smoser | http://paste.ubuntu.com/25485922/ | 20:21 |
| rharper | and the bug we're fixing today that's "cloud-init" fault in artful on non-intel | 20:21 |
| smoser | rharper, wel... if we logged and moved forward there. | 20:21 |
| smoser | we'd have just kicked the can well down the path | 20:21 |
| smoser | and never known about the fact that these instances were now using the "mostly functional" ec2 datasource when it owuld have been better to use the openstack one | 20:21 |
| rharper | I know; there's no good answer if we're not going to go and fix it ourselves | 20:21 |
| smoser | which was the original design goal | 20:22 |
| rharper | the busted network config (And using fallback) can have a banner just like ds-identify did | 20:22 |
| smoser | well, one way or another we need to have this fixed today. the fix that is there irght now will solve the bug | 20:23 |
| smoser | and my today is running short | 20:23 |
| blackboxsw | https://bugs.launchpad.net/cloud-init/+bug/1715738 filed | 20:27 |
| ubot5` | Ubuntu bug 1715738 in cloud-init "Cloud config modules are not skipped based on distro support" [Undecided,New] | 20:27 |
| rharper | I'm fine with a follow up | 20:27 |
| blackboxsw | I'm+1 on your approach either way. will review what you have again smoser. I was just throwing peanuts at the testing if isdict, but not a true concern | 20:28 |
| rharper | if I work on this: https://bugs.launchpad.net/cloud-init/+bug/1708255 | 20:28 |
| ubot5` | Ubuntu bug 1708255 in cloud-init "empty or invalid network config dictionaries are not handled well" [Undecided,New] | 20:28 |
| rharper | that'll help | 20:28 |
| smoser | ok. | 20:29 |
| smoser | i'm going to add a unit test that trips the ValueError | 20:30 |
| smoser | and then push with that patch most recent above. | 20:30 |
| smoser | man though. | 20:30 |
| smoser | that really would just kick the can and we wouldnt even know about this problem | 20:31 |
| smoser | so maybe i convince myself to just leave it as it is | 20:31 |
| asthasr | Good afternoon! Another question. I have put a script in /etc/cloud/scripts/per-boot (the pre-existing directory) while building an AMI. After standing up an EC2 instance based on this AMI, that script does not seem to run. Is there some config I have to set to make it happen? | 20:33 |
| smoser | asthasr, /var/lib/cloud/scripts/per-boot | 20:39 |
| asthasr | oh. | 20:39 |
| asthasr | what's the /etc one for? | 20:39 |
| smoser | the same thing as /etc/i/dont/know/why/you/have/that/directory | 20:40 |
| smoser | :) | 20:40 |
| asthasr | oh, sorry, i misspoke. I'm actually putting them in /var/lib/cloud/scripts/per-instance. Long day :p | 20:40 |
| smoser | asthasr, the module 'scripts_per_instance' should run those | 20:41 |
| smoser | i'll do a quick test | 20:41 |
| asthasr | my workflow is to use packer.io, which templates that script in, and then gives me an AMI. I then use terraform to build an instance based on that AMI | 20:42 |
| smoser | rharper, :-( | 20:42 |
| smoser | $ lxc init ubuntu-daily:xenial x2 | 20:42 |
| smoser | Creating x2 | 20:42 |
| asthasr | my expectation would've been that the script runs and I see the resulting test file when I log into the new instance | 20:42 |
| smoser | $ mount-image-callback lxd:x2 mchroot /bin/bash | 20:43 |
| smoser | lxd:x2: no rootfs in /var/lib/lxd/containers/x2/rootfs. Not a container? | 20:43 |
| rharper | smoser: whoa | 20:43 |
| rharper | =( | 20:43 |
| smoser | i think the path now doesn't get created until 'start | 20:43 |
| rharper | boo | 20:43 |
| rharper | issue time | 20:43 |
| rharper | hrm, working for me on xenial | 20:44 |
| rharper | smoser: what lxd do you have ? | 20:44 |
| rharper | I'm on 2.0.10-0ubuntu1~16.04.2 | 20:44 |
| smoser | $ dpkg-query --show lxd | 20:44 |
| smoser | lxd2.17-0ubuntu2 | 20:44 |
| smoser | i noticed it a while ago | 20:45 |
| smoser | i'll file an issue, but i dont have high hopes | 20:45 |
| rharper | well, it's pretty critical to our integration testing on lxd no ? | 20:45 |
| asthasr | well, I'm dumb. It needs to be u+x or it won't run. | 20:48 |
| asthasr | :) | 20:49 |
| rharper | asthasr: was there any log about it not being u+x ? | 20:49 |
| rharper | I think scripts-per are executed via cloud-init invoking subprocess on 'runparts' | 20:49 |
| rharper | run-parts; hopefully we caught some error there | 20:50 |
| asthasr | rharper: not that I see. | 20:50 |
| asthasr | saw, I should say; I didn't spot any errors. Unfortunately I already killed the instance. | 20:51 |
| rharper | if you recreate, would be good to get the cloud-init.log (and cloud-init-output.log) and file an issue; it'd be helpful to see an error in the logs for something like that I think | 20:51 |
| smoser | https://github.com/lxc/lxd/issues/3784 | 20:52 |
| asthasr | rharper: Understood. Will try tomorrow. | 20:52 |
| smoser | rharper, we have a runparts function. does not use the utility | 20:53 |
| smoser | it does not log anything on non-executables | 20:53 |
| rharper | indeed | 20:54 |
| smoser | uploaded ubuntu/0.7.9-267-g922c3c5c-0ubuntu1 | 21:04 |
| blackboxsw | smoser: are we SRUing that? | 21:16 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!