[11:25] <otubo> smoser, can you confirm if this bug has been addressed? https://bugs.launchpad.net/ubuntu/+bug/1225922 I can't track anything on the commit log that could relate to this
[12:44] <rick_h> johnsonshi:  I don't think we've got anything against it. We haven't had a chance to look at the change yet but did discuss it and don't have any reason not to.
[13:30] <smoser> otubo: i sure would think that is fixed.
[13:34] <smoser> replied in the bug
[13:44] <otubo> smoser, thanks a lot! Also there was another issue that - if you have some time to spare - I'd like you to check: https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+ref/fix/1788915-vlan-sysconfig-rendering
[13:44] <otubo> smoser, I also couldn't track on the commit log if this was merged or not. We might have an issue in RHEL that might benefit from this patch
[13:54] <paride> smoser, I think `origtargz --download-only --tar-only` could be used instead of get-orig-tarball in build-package
[13:55] <paride> it's included in devscripts
[13:55] <paride> but it's doesn't do some tricks implemented by get-orig-tarball
[13:56] <paride> like `get-orig-tarball for ...`
[13:56] <smoser> thats the first i'd seen of that.
[13:56] <smoser> a few things i like better from get-orig-tarball:
[13:57] <smoser>  doesnt need apt source lines
[13:57] <smoser> things i like in origtargz
[13:58] <smoser>  uses pristine-tar (that'd be really cool if it worked with git-ubuntu pristine-tar)
[13:58] <smoser>   that was something i wanted to add to get-orig-tarball
[13:58] <smoser>  - uses uscan (but --download-current-version isnt sufficient)
[14:04] <paride> and uscan it not guaranteed to fetch the .orig that was used for the packaging
[14:04] <paride> for example in some packages I maintain I use uscan to get notified about new upstream releases
[14:05] <paride> but then I import the upstream git tag and generate the orig tarball from my local git tree
[14:05] <paride> with gbp-buildpackage or `git deborig`
[14:09] <smoser> yeah.
[14:09] <smoser> i find it surprising/terribly-annoying that there isn't a "right way" to do this.
[14:10] <smoser> and a slew of wrong ways that will get your upload rejected
[14:15] <rharper> otubo: smoser: on that vlan-sysconfig bug;  I had someone else bring it up but I've not been able to get a reliable reproducer (I don't have a real host with bonded vlan and switches (where the bond may takes some time to come up);  in particular from a sysconfig perspective, there was some mention of use of ONPARENT=yes ;   so in the case of RHEL7.latest images which have NM only (with the sysconfig helpers)  is there
[14:15] <rharper> documentation on what should be present in the sysconfig file to do bonded vlans?    https://bugs.launchpad.net/bugs/1888315
[14:17] <smoser> otubo: wrt https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/366602 ... it just got dropped.
[14:17] <smoser> i dont have anything local more than what is there.
[14:19] <smoser> imo 'a' really isn't OK.
[14:19] <smoser> that is just slogging on an pretending stuff is happy, hiding original failures.
[14:19] <smoser> and doing something seemingly randomly different.
[14:20] <smoser> but... doing it right (b) is harder, and failing (c) probably breaks someone's idea of "working"
[14:30] <otubo> rharper, I'm not aware if there's a direction if we should use this option or not, but on the BZ[0] I have the reporter that might be willing to try the original patch from smoser. If that fixes the issue I might send a new PR with that code.
[14:30] <otubo> [0] https://bugzilla.redhat.com/show_bug.cgi?id=1861871
[14:31] <otubo> smoser, right, I'll give it a try, see if it really fixes the issue first. We'll see from there.
[14:32] <otubo> *might be willing to try, meaning, the reporter has the hardware and the environment necessary :-)
[14:36] <rharper> otubo: well, I believe it does resolve an issue; but I don't understand why;
[14:38] <rharper> The other alternative w.r.t sysconfig and cloud-init  is to emit NM output;  for example in RHEL8, right, NM is the default now;  is the sysconfig compat layer still around?  If the sysconfig files are still supported; then we should figure out why what we currently write doesn't work for bonded vlans;  it's certainly a regression between sysconfig-only setups (where this code works perfectly fine) and NM-only setups which import
[14:38] <rharper> sysconfig files.
[15:24] <smoser> well, the right behavior is to render what the system is configured for.
[15:24] <smoser> if it is NM, then render NM. if it is sysconfig, then sysconfig.
[15:53] <rharper> we don't render NM;  AFACIT NM supports reading sysconfig files;  so yes, someone can write a NM renderer for cloud-init;  even without cloud-init though, either the sysconfig file we're writing is valid or invalid; and I'd like to understand why it works without NM; but when NM imports it; it doesn't;
[16:38] <momousta> blackboxsw_, We would like to land this PR https://github.com/canonical/cloud-init/pull/529 as part of the next SRU, Can you take a look?
[16:38] <blackboxsw_> momousta: yes thanks for the piing
[16:38] <blackboxsw_> will look today
[16:38] <blackboxsw_> just got back from "vacation": )
[16:39] <blackboxsw_> momousta: I was wondering whether we want to make that filtering a bit more generic. I'll think a bit this morning and get you a concise response and/or review today
[16:54] <blackboxsw_> momousta: could you run rebase your current PR  and resolve merge conflicts please? `git fetch -i origin;  git rebase -i origin/master; git push <your_remote>`  might do it.
[16:54] <momousta> Sure
[16:54] <blackboxsw_> thanks
[16:55] <blackboxsw_> sorry about that git fetch -i origin; just git fetch origin;
[16:57] <blackboxsw_> thx
[17:55] <cut> is there any way to silence cloud-init so it doesn't print the "Created by.." message in various files in /etc/? "# Created by cloud-init on instance boot automatically, do not edit."
[18:32] <blackboxsw_> cut, not by any configuration setting. you'd have to do post-processing on those files, something like " #cloud-config\nruncmd:\n - "sed -i 's/Created by cloud-init on instance boot automatically, do not edit./Silence safety message/' /etc/<your_file_path>.
[18:33] <cut> thanks
[18:57] <minimal> Hi folks. I saw the email about the planned release of 20.3 on Thursday. I wanted to check if there's a chance to get my PR #535 into this release. I'm currently working on changes based on the last set of review comments and hope to get them committed in the next couple of hours time.
[19:00] <rharper> minimal: I'll review again
[19:01] <minimal> rharper: Thanks
[19:07] <blackboxsw_> momousta: only one significant note so far on your branch it looks like with every boot you'll end up dumping the full compressed cloud-init.log for every boot across kvp, that's going to get more and more costly the more times an azure instance is rebooted. https://github.com/canonical/cloud-init/pull/529/files#r472392050
[19:08] <momousta> blackboxsw_. I'm looking into that now. Thanks for the quick feedback.
[19:09] <blackboxsw_> I'm not sure, but you may want to think about persisting something with that byte-offset on disk somewhere in /var/lib/cloud/data/   like "azure-kvp-offset" if you ultimately want to continue to always log updated compressed cloud-init.logs per-boot
[19:09] <blackboxsw_> no prob momousta
[19:11] <rharper> minimal: I did review yesterday, did you see the comments about the ntp test-cases?
[19:25] <minimal> rharper: yes I saw your review yesterday - I'm working on the changes for it currently. Hope to have them committed shortly.
[19:27] <minimal> rharper: re: the cc_power_state_change.py re.match change I did add a note about 30mins ago to explain why I'd done that. I've since added "\s*" (zero or more whitespace) to the start and end of the regex pattern, that'll be committed shortly.
[19:32] <rharper> minimal: great;  yeah; it looks like int() eats leading +,- and any number of whitespace (or trailing);  I just commented; my preference is to not change the code even if the docs don't match since if there's a cloud-config using something that passes the current regex; I want that config to still work rather than break because we tighted it;  OTOH, I don't think there's much else that int() won't catch besides + - and spaces
[19:32] <rharper> (which it ignores anyhow;
[19:48] <minimal> rharper: well the main thing I wanted to trap was someone trying to use '2:30' to shutdown in 2.5 hours time - this is valid for shutdown command if passed as a param in that format but doesn't match the docs for cc_power_state_change
[19:51] <rharper> minimal: yeah, the exception message is nicer than the int() failure to convert
[19:51] <rharper> but we could also print the same message in the exception for int()
[19:51] <rharper> ValueError: invalid literal for int() with base 10: '+30:30'
[19:52] <rharper> nm, they'll throw the error  before
[19:53] <rharper> that logic likely needs a refactor there;   it seems to me that we should handle now, and then try int() and except ValueError on the conversion and reraise as TypeError like it currently does if if fails;
[19:55] <rharper> minimal: https://paste.ubuntu.com/p/hQZtDR97Ft/
[19:56] <rharper> then with your branch changes on top of that (we get to drop the regex since int() already handles what we expect
[20:05] <minimal> rharper: ok, could go with that
[20:14] <minimal> rharper: actually I'm confused :-) The code you pasted don't reflect the last commit I made a couple of days ago when I added the convert_delay function as you suggested and the if-alpine-else-others block to call convert_delay. Should I simply change the regex pattern back to the way I was and left things at that?
[20:20] <rharper> minimal: sorry, I was just demonstrating the refactor around using the int() and exception so we could drop the regex;  I meant to have that change I posted integrated on top of your change;
[20:21] <blackboxsw_> thanks again momousta I just added a few more notes, minor concerns to this branch. I'll peek at this again when you say it's ready for re-review
[20:22] <rharper> minimal: so I think you can remove the try/except in convert_delay() and instead in the main function try: convert_delay() except: raise TypeError()
[20:30] <minimal> rharper: from memory the try/except in convert_delay() is needed as for non-Alpine distros a value 'now' can be passed in and therefore returned to be used in the shutdown command
[20:33] <rharper> minimal: yes, but also only calling convert_delay() if delay != 'now'
[21:12] <blackboxsw_> merged https://github.com/canonical/cloud-init/pull/428 thanks otubo !
[21:12] <blackboxsw_> That'll be in our next upstream release 20.3
[21:14] <blackboxsw_> community notice: As falcojr announced on the mailing list we are planning on cutting upstream release 20.3 thursday (and shortly thereafter starting an Ubuntu SRU process to release to Xenial, bionic and focal).
[21:15] <blackboxsw_> community notice: if there are existing branches that folks really would like in this time-based release. please raise them here or on the mailing list
[21:16] <blackboxsw_> on our list currently are the following PRs https://github.com/canonical/cloud-init/pull/529 ~https://github.com/canonical/cloud-init/pull/428~ https://github.com/canonical/cloud-init/pull/487
[21:16] <blackboxsw_> we would like to land these branches in tip before cutting an upstream  release 20.3
[21:17] <rharper> blackboxsw_: PR #535 is hopeful for it
[21:17] <rharper> minimal and I are currently working to land this for Thursday
[21:17] <blackboxsw_> #529 has a couple of review comments to discuss/address and I think we are good there on Azure.   #428 just landed   and #487 needs a little review love
[21:18] <blackboxsw_> rharper: checking that out thanks. and will add it to the list
[21:18] <minimal> blackboxsw_: yes I'm working on the remaining issues in PR #535 as we speak
[21:19] <blackboxsw_> excellent minimal/rharper. just holler if we are stalling out on that. I think we have (side-channel) about 3 prs to land for grub fixes in debian/postinst for ubuntu/* related branches.
[21:20] <blackboxsw_> I'm off on PTO for tomorrow (OddBloke is out all week), but falcojr and I will be trying to get through any outstanding reviews that are not already being handled by rharper and or smoser too.
[21:24] <rharper> blackboxsw_: ok
[22:14] <minimal> rharper: Hitting a problem with test_handler for cc_apk_configure - multiple test fails and all I suspect is the add_patch for write_file that you suggested I add
[22:15] <minimal> I'm seeing tox errors: E       FileNotFoundError: [Errno 2] No such file or directory: '/tmp/ci-TestConfig.gc5tegx6/tmp/template_name-povkwjvo.tmpl'
[22:16] <minimal> which is the temporary template file write by cc_apk_configure which it is then unable to load to do the config file generation from this template
[22:25] <rharper> minimal: yeah, we write a temparary template file and then write the content sof that
[22:26] <minimal> actually multiple unrelated tests across various modules are failing with the same error
[22:26] <minimal> so somehow the changes I made in test_handler_apk_configure are failing large numbers of tests across the board
[22:27] <minimal> 137 failed
[22:27] <rharper> minimal: if you dump what you have (git diff origin/master) and paste; I can help refactor some of the unittests;
[22:32] <minimal> its a big diff for the test_handler, approx 300-odd lines
[22:33] <rharper> or push what you ahve to a branch and I'll pull from github;
[22:37] <rharper> I've got what you've pushed locally; I can work on a refactor of what I suggested on the apk stuff;
[22:37] <rharper> bbiab
[23:05] <minimal> rharper: https://gist.github.com/dermotbradley/42e6f4cb7298498f9ce9a87d44800ed3
[23:40] <rharper> minimal: ok