=== cpaelzer__ is now known as cpaelzer
[08:51] <catphish> what does public_ssh_keys in the metadata do?
[09:04] <catphish> i think i'll just not worry about meta-data keys unless i find i actually need them
[09:37] <otubo> (sorry I was just disconnected from irc, will paste the messages again)
[09:37] <otubo> Odd_Bloke: hey, thanks a lot for the reivew! Regarding the commit message, I guess it is on the PR itself? it too big? Or do you need more details?
=== hjensas__ is now known as hjensas
[12:46] <smoser> c-i runs as root on travis ?
[13:21] <catphish> quick question - what's the correct way to have cloud-init only run once, specifically once my config has applied successfully i don't want subsequent boots to waste time looking for an updated configuration from the network
[13:21] <catphish> ideally i'd want this to be reversible so that if a machine is cloned, i could easily trigger it to run again, but only in that specific case
[13:23] <ananke> perhaps disable the systemd services as part of runcmd module
[13:25] <ananke> though I wonder if 'waste time' is actually a noticable problem
[13:26] <otubo> Odd_Bloke: OH, just noticed you might be referring to the PR template. Danm, sorry, my PR is old and we didn't have it back then. I'm gonna work on that.
[13:26] <catphish> actually, thinking about it, it would be even better if my datasource itself could compare the UUID of the machine with the one from the previous run, and return a dummy data set if they match
[13:27] <catphish> ananke: well because my datasource is network based, i don't really want subsequent boots to hang if there's a network problem, and the process of doing the network based config does delay the boot a little even when it's working correctly
[13:28] <catphish> i'll see if i can have my datasource do the work itself and compare the UUID from the BIOS with the one from the last run
[13:36] <smoser> catphish: what you're describing is basically how it works.
[13:38] <smoser> you could say specifically which datasource your're' talking about
[13:39] <smoser> but ec2, openstack (on intel/kvm) and others do that. (check_instance_id is what determines "is this new").
[13:41] <minimal> Hi folks. Is there any agreement on whether strings in user-data (i.e. YAML) should be single-quoted or double-quoted? Is see varying uses of both in the docs and in the testcases
[13:42] <catphish> smoser: that's ideal, do they pull the instance id from the bios for the check?
[13:42] <catphish> smoser: sorry, it's *my* datasource, i haven't pushed it yet
[13:43] <catphish> i'll go check check_instance_id anyway, thanks!
[13:43] <smoser> they compare the cached instance id to what is exposed by the platform in dmi data.
[13:43] <catphish> perfect, that's exactly what i want do do, i'll go and find that code
[13:43] <smoser> buton those platforms the platform states taht instance-id == <that-value>
[13:44] <smoser> we're not just comparing serial number or some other field.
[13:45] <catphish> i'm not sure i understand, but perhaps i will if i read the code
[13:45] <smoser> the platform (ec2 and openstack) state that they expose the instance-id through dmi
[13:49] <catphish> looks like check_instance_id will do what i want, in openstack it calls instance_id_matches_system_uuid
[13:51] <catphish> digitalocean calls: instance_id_matches_system_uuid(self.get_instance_id(), 'system-serial-number') which is close to what i actually want, thank you!
[14:18] <Odd_Bloke> otubo: Yeah, only asking because I want to be sure that the words attached to your name in the squash commit are the ones you would choose. :)
[14:20] <otubo> Odd_Bloke: ACK on the text :-)
[14:25] <Odd_Bloke> minimal: There is not, to my knowledge.
[14:27] <Odd_Bloke> Both quote types are valid and have different uses (per the YAML spec: https://yaml.org/spec/1.2/spec.html#id2786942), primarily to do with escaping.
[14:27] <Odd_Bloke> (But it sounds to me like single-quoted for preference, and double-quoted if necessary makes sense.)
[14:29] <minimal> Odd_Bloke: ok. Am working on a PR to revise/correct docs for a particular issue where a value needs to be quoted and thought it would make sense for all relevant testcases and docs to use the same notation.
[14:32] <minimal> One of the examples I want to correct is macaddress value, it must be quoted (either single or double) but the docs don't indicate this and there's only 1 testcase where its quoted (single in this case). So I need to decide whether to specify single or double in my PR
[14:33] <minimal> So I'll go with single quote then.
[14:34] <Odd_Bloke> Yeah, that sounds good to me!
[14:35] <minimal> have found quite a few minor user-data issues that the current testcases don't pick up on
[14:37] <otubo> Odd_Bloke: also, is it ok to "Update branch" from the Github interface? Does it generate garbage upon merge?
[14:37] <minimal> another question, is regarding the User & Groups module, the setting "inactive" doesn't make sense. The docs state: "Mark user inactive. Default: false"
[14:37] <Odd_Bloke> otubo: We'll do it if necessary before merge, but it's totally fine for you to do too.
[14:37] <otubo> Thanks.
[14:38] <minimal> However according to "useradd" docs for "--inactive" it is used for "The number of days after a password expires until the account is permanently disabled." which is not the same thing
[14:41] <Odd_Bloke> minimal: Yeah, that seems like a clear issue: it _looks_ like the code will currently allow you to pass a string and it will DTRT, so that may just be a documentation issue at least?
[14:43] <minimal> I mean the c-i docs define inactive as a boolean whereas the code when creating the user for most OSes calls "useradd --inactive" with no value which means the value in /etc/default/useradd is used, which is "-1" which means disable the inactive feature - end result, defining inactive: true in user-data in reality does not use the inactive feature
[14:44] <minimal> typically the useradd/usermod inactive feature takes a value for numeber of days
[14:51] <minimal> Odd_Bloke: yeah I'd expect the user-data inactive to take days as a param. However FreeBSD maps this to "pw useradd -E" and its not clear if for FreeBSD if this is a boolean or takes a number as well (only online pw man I've found doesn't document -E)
[15:05] <minimal> Odd_Bloke: just retested to double check my notes - inactive: '5' works but inactive: 5 (without any quotes) does nothing
[15:07] <Odd_Bloke> minimal: Right, the code checks specifically for a string; if the value is one then it will add `["--option", "value"]` to the command line, if it _isn't_ a string (i.e. a bool as documented, or an int as you just tried) then it adds only ["--option"].
[15:07] <Odd_Bloke> That int fact is definitely going to catch people out, no matter how we change the docs.
[15:08] <minimal> Well my concern is I don't know how FreeBSD acts and so if I update the docs (and testcases) to expect a string with a number of days will it break FreeBSD?
[15:09] <minimal> also the only testcase I could find that specified inactive doesn't actually check/simulate /etc/shadow to verify if it acts as expected
[15:09] <Odd_Bloke> https://github.com/canonical/cloud-init/blob/master/cloudinit/distros/__init__.py#L473-L484 is the code I'm referring to, FAOD.
[15:09] <Odd_Bloke> Yeah, that's a great Q about BSD too.
[15:10] <minimal> yeah, __init__.py is used for *most* OSes but not BSD, freebsd.py passes "-E" to "pw user" instead
[15:11] <Odd_Bloke> I can't find -E but https://www.freebsd.org/cgi/man.cgi?pw(8) does have `-e`: "Set the account's expiration date."
[15:12] <minimal> yeah, same page I found. That's why I couldn't work out what -E does (or is it a typo?)
[15:12] <Odd_Bloke> It looks like -E was introduced when FreeBSD support first landed in 2013, so it could either be a typo or something that has changed since then.
[15:13] <Odd_Bloke> meena: Goneri: Are either of you able to weigh in?
[15:13] <Odd_Bloke> minimal: (They're our two most active *BSD contributors. :)
[15:13]  * Goneri reads
[15:13] <minimal> if its meant to be "-e" then we're in even more of a bind as that takes a date string rather than a number of days
[15:14] <minimal> hmm, it does take a number of days if "+" prefixed.......
[15:19] <Goneri> to disable an account we should probably do something like that: pw mod user root -w no
[15:21] <Goneri> I remember on BSD you've got a case where the account is disabled and another one where it's just the password. I cannot really elaborate on the, but I remember it was rather different comparing with Linux.
[15:23] <Goneri> also on NetBSD and OpenBSD, you cannot enable/disable a password. You instead need to nuke the password and to restore the PW auth, you need to reset a new PW.
[15:24] <minimal> well I guess that's the underlying question - what is the "inactive" setting supposed to achieve? Is it meant to disable an account (in which case current functionality does not achieve that - the useradd --inactive flag and the underlying /etc/shadow field control "The number of days after a password has expired during which the password should still be accepted"
[15:25] <minimal> disabling password is expected to be controlled by "lock_passwd" user-data setting
[15:26] <Goneri> Good question, my understanding is that we just disable the pw auth. We can face some surprise otherwise, fFor instance if we disable the root password, and we actually really kill it.
[15:27] <minimal> Goneri: I mean there are 2 distinct user-data settings for users: "inactive" and "lock_passwd", what you're describing should be controlled by "lock_passwd"
[15:27] <Goneri> it's more a question for the core team.
[15:27] <Goneri> what is the meaning of the inactive flag
[15:28] <Goneri> and we will probably need to audit the current code base, to ensure we match the expectation.
[15:29] <minimal> I expect these options were created by looking at the various options that useradd and usermod take and mapping to them
[15:30] <minimal> so useradd has an "--inactive" option and usermod has a "--lock" option (this just puts a "!" in front of the password)
[15:31] <minimal> locking account for useradd/usermod is controlled by "--expiredate" which maps to user-data "expiredate"
[15:34] <minimal> Odd_Bloke: so I think the current situation is that for user-data "inactive" the FreeBSD code should use "-e" instead of "-E" and it should accept a string containing a number of days and then map that to calling "pw useradd -e +VALUEd"
[15:36] <minimal> with this in mind I can update the docs to indicate that "inactive" takes a string of number of days and someone (trying not to volunteer myself ;-)) can update the current testcase that specifies "inactive" to actually also simulate /etc/shadow file and check that the value is placed in there correctly
[15:43] <minimal> there's no schema defined currently in the cc_users_group.py file so nothing to fix in that
[15:43] <Goneri> I'm glad we've got a volonter
[15:43] <Goneri> :-)
[15:57] <Odd_Bloke> In terms of 'what is the "inactive" setting supposed to achieve?', it's intended to allow users to specify `--inactive <value>` to the useradd call that happens for this user; our docs (and behaviour) should be aligned with `--inactive`s actual interface and behaviour.
[15:57] <Odd_Bloke> "I expect these options were created by looking at the various options that useradd and usermod take and mapping to them" <-- aha, yep, exactly that
[16:00] <minimal> I'll revise the description for "inactive" to reflect the reality
[16:02] <Odd_Bloke> https://www.freebsd.org/cgi/man.cgi?query=pw&apropos=0&sektion=8&manpath=FreeBSD+9.3-RELEASE+and+Ports&arch=default&format=html (note this is for FreeBSD from ~2013) also does not mention -E, so I think that must just be a bug?
[16:02] <Odd_Bloke> Goneri: Do you know(/can you check) if `-E` will work at all with `pw useradd`?
[16:12] <minimal> BTW there's no user-data setting for defining a user's password expiry date. "expiredate" controls when the account is disabled, not when the password expires.
[16:14] <minimal> I guess because useradd/usermod don't appear to have options to control this (it managed by /etc/default/useradd and the user's /etc/shadow entry)
[16:40] <Goneri> Odd_Bloke, the flag does not exist.
=== ijohnson is now known as ijohnson|lunch
[17:37] <Odd_Bloke> Thanks!
[17:37] <Odd_Bloke> minimal: So given ^, we may also want to document that the option does not work for FreeBSD.
[17:40] <meena> well, crap
[17:41] <meena> can we just outright reject it on (Free?)BSD?
[17:48] <blackboxsw> paride: I approved https://github.com/canonical/cloud-init/pull/576 for this iteration as I think we can address the cpick issue when we next hit it if we have a need to test locally during our next release freeze. rharper is there still something that you wanted to block for this pass on bddeb?
=== ijohnson|lunch is now known as ijohnson
[18:03] <minimal> Odd_Bloke: if freebsd.py is changed to use "-e +DAYSd" then it should work as well
[18:08] <Odd_Bloke> Yeah, that would work.  A bit of me wonders if we should allow FreeBSD users to pass whatever they want to `-e` (i.e. expect them to pass +2d if that's what they mean), so that they aren't restricted to only using the subset of expiration functionality that's supported on Linux.
[18:09] <Odd_Bloke> But then it means that user config is less portable between Linux and BSD, which is also less than ideal.
[18:10] <Odd_Bloke> I suppose that if someone wants some of the BSD-specific functionality then they can always use a runcmd instead.
[18:17] <rharper> blackboxsw: no; I'm fine with bddeb
[18:48] <johnsonshi> Odd_Bloke blackboxsw: Hi folks, I'm looking to get https://github.com/canonical/cloud-init/pull/594 in by the next SRU. The PR adds "report_failure" functionality (description in the PR). The changes in that PR are fairly short (mostly changes in DataSourceAzure.py and the azure.py helper). Most of the lengthy additions comes from additional UTs I added for the report failure functionality.
[18:48] <blackboxsw> thanks rharper.
[18:49] <blackboxsw> johnsonshi: excellent. we talked about reviewing this PR at today's standup meeting.  So, we expect to close it out shortly and we'll make sure it's in for the next SRU.
[18:49] <johnsonshi> I've performed deployment tests to ensure that (1) there are no regressions, and (2) that the report failure functionality works. Also added lots of UTs covering each possible case. I think the PR should be ready now :)
[18:49] <blackboxsw> which I think is before end of year.
[18:49] <johnsonshi> blackboxsw: Thanks!
=== MAbeeTT_ is now known as MAbeeTT