2022-12-27T03:34:42 *** teepee_ is now known as teepee 2022-12-27T16:41:18 *** teepee_ is now known as teepee 2022-12-27T20:24:13 Hi cboltz, GitLab throws 500 upon creation of a new MR. Hence I am asking you the old school way, if you find some time, to review my branch "lock-module" which would help me as a base for an upcoming deployment state. 2022-12-27T20:24:21 not urgent whatsoever 2022-12-27T20:25:11 nice[tm] - and I already had hoped that the new version I installed some days ago would at least not break more things... 2022-12-27T20:26:41 it's a box of surprises at this point 2022-12-27T20:32:23 looking at your branch - 2022-12-27T20:33:23 - fixed filenames in /tmp/ ? Even if you "just" touch and rm these files, I don't really like that. Maybe use a directory where only salt/your script has write access? 2022-12-27T20:34:08 - several tests check for is_file() - but what happens if it exists, but is for example a directory? 2022-12-27T20:34:45 I didn't quite like it either, what about make temp directory an argument with /tmp/ being the default? 2022-12-27T20:35:34 hm I guess it being a directory would be fine to treat the same way as already exist? 2022-12-27T20:36:02 that probably still means /tmp/ in most cases (I mean, who changes defaults if they work?), I'd prefer a better location as default 2022-12-27T20:36:32 I'm not sure what would universally work, /var/lib/salt/ doesn't always have to be there either 2022-12-27T20:36:42 the main question is how the check responds to a directory ;-) - and note that there are also symlinks, fifos, device nodes, ... 2022-12-27T20:37:44 well, if /var/lib/salt doesn't exist, I'd prefer an error over using an insecure directory ;-) 2022-12-27T20:37:45 well is_file only returns true on regular files or symlinks 2022-12-27T20:38:17 I guess that's fair. but I wonder if there's a variable for something like $salt_working_directory, 2022-12-27T20:38:48 good question, unfortunately I don't have an answer ;-) 2022-12-27T20:38:52 :-) 2022-12-27T20:39:11 will ask about it 2022-12-27T20:39:23 for is_file - that also means all the other cases (directory, device node etc.) will give your script a somewhat wrong behaviour because it will think "doesn't exist" 2022-12-27T20:40:01 using a trusted directory will mostly fix this because you can expect that only salt puts stuff there, and isn't evil 2022-12-27T20:41:13 good points, but I can just change it to exists(), which returns true on any kind of "thing". the error message is ambigious but as you say that should not be an issue with a better directory 2022-12-27T20:41:30 right 2022-12-27T20:42:20 on a more general question - what's the usecase for this file? 2022-12-27T20:43:51 I wrote an Orchestration state which deploys virtual machines, it avoids two administrators trying to deploy the same VM simultaneously 2022-12-27T20:44:07 there are probably more elegant ways but this "works" 2022-12-27T20:45:41 BTW: it looks like creating the MR somewhat worked - I get 500 when trying to view it, but OTOH I got some notification mails for it 2022-12-27T20:46:45 I was surprised to receive pipeline notifications which I could actually visit the links of 2022-12-27T20:47:12 if I was motivated to dig into GitLab, I'd try to review my MR through the API, but I am not :P 2022-12-27T20:47:41 the pipeline runs in your branch, not in the MR 2022-12-27T20:48:47 oh right we don't use MR pipelines 2022-12-27T20:53:25 updated (squashed) 2022-12-27T20:59:44 looks much better :-) 2022-12-27T21:01:51 one thing I wonder is - what happens if creating or deleting the lockfile fails? 2022-12-27T21:02:14 I'd guess that you get an exception 2022-12-27T21:04:04 just tested - unlink can result in FileNotFoundError or PermissionError or IsADirectoryError (and maybe more for things I didn't test) 2022-12-27T21:04:15 if it's an exception returning False in Pathlib it should return False with "failed to create lockfile $name", otherwise some stacktrace 2022-12-27T21:04:41 yea that sounds about right 2022-12-27T21:05:07 touch() can at least cause a PermissionError if a file exists, but is owned by someone else 2022-12-27T21:05:42 also, it succeeds if the file already exists 2022-12-27T21:05:56 you check for non-existence before, but this could still be a race condition 2022-12-27T21:05:57 also remember that apparmor violations result in EACCESS instead of EPERM from the kernel. 2022-12-27T21:06:15 that success is expected, because it's a Salt state you define "this file should be present" 2022-12-27T21:06:55 right, but since it's meant as lock file, I'd expect "this file was just created _by me_ and is now present" 2022-12-27T21:08:12 also, deleting lock files is troublesome for proper locking: https://bugzilla.suse.com/show_bug.cgi?id=864785 2022-12-27T21:09:06 hmm that's sort of what lock.check does, here is how I intend to run it https://paste-test.opensuse.org/pastes/bfdd6c6c5ce3 2022-12-27T21:10:15 I agree it's not ideal bmwiedemann[m] but then again it shouldn't be used as a reference to lock database files or the like :-) 2022-12-27T21:12:00 it's mostly as a foolproof measure for people not checking `w` 2022-12-27T21:12:35 for the "I just created it" part - in a shell script, I'd use mkdir (without "-p") instead of touch to get this behaviour because mkdir will fail if the directory already exists (I'd have to check if python also behaves this way) 2022-12-27T21:12:56 (and I admit that a race condition between two admins is less likely than between two programs ;-) 2022-12-27T21:13:58 I assume that's what the `exist_ok` argument does to path.exist() 2022-12-27T21:15:45 https://paste-test.opensuse.org/pastes/562d5eb5aadb 2022-12-27T21:16:38 indeed, using that argument looks like a good idea 2022-12-27T21:37:23 what about now? 2022-12-27T21:37:39 + sorry, should have kept extra commit for easier diff 2022-12-27T21:38:44 no problem, I copied the file locally before git pull ;-) 2022-12-27T21:39:04 looks good 2022-12-27T21:40:04 I'd also use try/except when calling unlink 2022-12-27T21:41:19 good idea 2022-12-27T21:41:46 it's good to see paste-test in use 2022-12-27T21:42:22 yeah, that confirms that it works :-) 2022-12-27T21:43:10 we will see tomorrow 2022-12-27T21:43:30 would it be possible to display the pastes in full width? With long lines, I get a horicontal scrollbar, and at the same time quite some whitespace right and left of the textarea 2022-12-27T21:45:25 probably, I would probably make it an option to expand the view on a click of a button somewhere 2022-12-27T21:47:30 personally, I'd just change all the .container-* to max-width: 100% ;-) 2022-12-27T21:51:25 I installed hellcp's CSS width overwrite snippet in Firefox 2022-12-27T21:52:00 all these years they made monitors bigger and bigger and then UI design finds out people find smaller width more comfortable 2022-12-27T21:53:48 well, it depends on the content (and nobody stops you from making your browser window smaller) 2022-12-27T21:54:22 (at least) if there is a horicontal scrollbar, increasing the width sounds like a good idea to me 2022-12-27T22:04:11 that's fair, for text I sort of generall prefer it to fill my screen, except if I work side by side on something 2022-12-27T22:04:26 module now with further exception catches :) 2022-12-27T22:05:44 already noticed it, looks good 2022-12-27T22:06:46 in test_extension.sh, you added _modules and _states to the regex for obvious reasons 2022-12-27T22:07:22 but it's less obvious why you changed README\.md to various extensions (.md, .yaml, .jinja, .j2), so - what's the reason for this? 2022-12-27T22:16:57 thanks for spotting this, it happened while carelessly copying what I had at work, the lines looked similar at first .. it's repaired now 2022-12-27T22:21:53 actually, that's not quite right 2022-12-27T22:23:13 now.. 2022-12-27T22:30:48 I made the nginx body size limit 20M now, just so that the app shows the correct error when uploading files larger than 2MB 2022-12-27T22:31:02 kinda frustrating there's no better way to do this 2022-12-27T22:31:42 (the default nginx limit is 1mb, which also is a bit low) 2022-12-27T22:33:53 so if I [try to] upload a 21 MB file, I'll get a wrong error message? ;-) 2022-12-27T22:35:33 you won't get an error message 2022-12-27T22:35:46 you will be redirected back without one 2022-12-27T22:37:39 oh, nice - but good to know once we get tickets about this (and I'm quite sure we _will_ get such tickets saying "doesn't work, just redirects me") 2022-12-27T22:39:10 I don't wanna have the app hammered with requests that have gigantic request body size that it won't handle anyway, I think this is not the worst compromise 2022-12-27T22:39:32 technically would be possible to handle body size checking in js, I just have to implement that 2022-12-27T22:40:13 hehe, "just" ;-) 2022-12-27T22:41:01 Jacob Michalskie: BTW, do you have an estimate/ETA for how long the migration will take? Should we add an incident in status-o-o? 2022-12-27T22:41:41 acidsys: now you lost the $ for (pillar\.example|\.sls|/README\.md) 2022-12-27T22:41:46 technically it's just changing the hostname in nginx and switching what anna points at with haproxy 2022-12-27T22:41:53 :-( 2022-12-27T22:42:03 so it can probably take under a minute 2022-12-27T22:42:26 will it? knowing me probably not 2022-12-27T22:43:07 acidsys: that's what you get for asking _me_ to do the review ;-) *eg* 2022-12-27T22:43:18 of course login won't work in that case, we will need somebody to update oidc metadata with the correct info 2022-12-27T22:43:36 ;-) I appreciate it 2022-12-27T22:43:50 I wouldn't call login essential tho 2022-12-27T22:44:28 why update oidc metadata, will we not have a separate prod instance 2022-12-27T22:44:50 or do we just move paste-test to paste 2022-12-27T22:45:04 oidc includes paste-test url, doesn't it? 2022-12-27T22:45:21 yes I thought we will have a second for paste 2022-12-27T22:45:21 in return uri? 2022-12-27T22:45:31 no need tbh 2022-12-27T22:45:37 ok 2022-12-27T22:46:30 well you can ping whenever you want to do it 2022-12-27T22:46:39 sure 2022-12-27T22:47:05 I just realize I never tried the login feature - I just open it and paste my wall of text 2022-12-27T22:47:31 it's currently not particularly useful 2022-12-27T22:47:44 what can I do with the openid_connect key ? 2022-12-27T22:47:48 * Luciano[m] tested the login and it works ™️! 2022-12-27T22:47:52 it does let you remove pastes 2022-12-27T22:48:09 oh but all additional ones I add also let me remove the pastes? 2022-12-27T22:48:11 and if you use the key, you can paste under your account 2022-12-27T22:48:24 interesting 2022-12-27T22:48:42 the paste history is useful though, I often find myself re-pasting things I forgot the link of 2022-12-27T22:48:52 it also shows you your private pastes in the paste list 2022-12-27T22:48:53 indeed 2022-12-27T22:50:34 acidsys: looks good now :-) - merged 2022-12-27T22:50:39 noticed, thank you! 2022-12-27T22:50:42 I do wanna write a new version of susepaste script that makes better use of the api that is now there 2022-12-27T22:51:39 I did start making a ruby version, but I don't know if pulling that makes sense for a script like that 2022-12-27T22:53:32 a bash script with some curl may end up making more sense 2022-12-27T22:56:28 YaST2 Paste Module 2022-12-27T22:57:07 🤔 2022-12-27T22:57:22 good idea, but it has to use a c++ library, maybe libpaste-ng? ;-) 2022-12-27T22:57:25 let's integrate paste-o-o into spectacle 2022-12-27T22:58:33 it would be cool to make it integrate with the kde "share" menu, so it will show in spectacle but other K-tools as well 2022-12-27T22:59:04 I wanted to add a generic curl-style uploader for this at some point but it never happened 2022-12-27T22:59:35 hm, interesting 2022-12-27T23:11:13 OK, for mental clarity. I know that we have VMs that are not in Icinga, but do we have VMs not Salt'ed that should be (I'm aware of the Cachet ticket in progress)? 2022-12-27T23:12:08 VM's should all be in Salt, but the Icinga part is not Salted 2022-12-27T23:12:21 do you have root access to the salt master, you could check salt 2022-12-27T23:12:28 s/salt/salt-key -L there/ 2022-12-27T23:17:09 I don't think so. I was just wondering because I counted the number of files in salt/pillar/id, and the number of VMs in "List of machines" (from the wiki) and got different numbers. But then I saw we have some testing machines here and there that are not public. 2022-12-27T23:17:26 oh right, checking pillar/id/ should work equally well 2022-12-27T23:17:35 the wiki is unfortunately extremely outdated 2022-12-27T23:18:23 Yeah, keeping dozens of items updated is not fun. 2022-12-27T23:18:59 So my concern is only Icinga then, thank goodness. 2022-12-27T23:19:44 this specific page is basically copy&pasting the output of a script that translates pillar/id/* to a table, so you "just" have to run the script and copy over the result 2022-12-27T23:19:51 templating the basic monitor files for Salt should be relatively easy, but some have additional perks for various services they include 2022-12-27T23:20:12 (I usually grep over pillar/id/ instead, but there are rumors that some people really read that wiki page instead) 2022-12-27T23:21:46 Hey, it was my primary source of truth. 2022-12-27T23:22:40 technically the wiki pages are a good idea to keep some machine-specific notes, but then again many never get updated 2022-12-27T23:23:42 Oh, so there's bin/generate_redmine_page.pl 2022-12-27T23:24:25 Yeah, the wiki page has some additional info, but could be a trap. 2022-12-27T23:24:46 but it* 2022-12-27T23:27:01 Now, I'm wondering if it would be a better idea to handle the Icinga Salt part to adding new VMs now. 2022-12-27T23:31:08 a profile.monitor or similar 2022-12-27T23:32:38 I was thinking if there's a way to "include" external files in this one could just template the basic things and allow some local overrides for now. but otherwise it would need some pillar for the various extra services, interfaces, etc 2022-12-27T23:40:16 we just need to make the ci deploy the wiki automatically 2022-12-27T23:40:41 OK, for now I'm just gonna add the poor VMs that were left out from Icinga - which shouldn't be many, from what I'm seeing - and then we can proceed from there. 2022-12-27T23:41:14 Heh, that'd be interesting, letting the ci do the boring job. 2022-12-27T23:41:31 that's what it should do 2022-12-27T23:41:59 I was kind of hoping we could do ci like that on pagure 2022-12-27T23:42:47 I don't think there's a single thing that's currently hosted on pagure's pages thing 2022-12-27T23:44:18 Apart from tracking Leap features, I honestly can't tell what else people are doing on code.o.o. 2022-12-27T23:44:59 Oh, and handling membership things. 2022-12-27T23:46:45 gitea somehow has even less things happening on it 2022-12-27T23:47:06 the more forges we have deployed the less any of them make sense 2022-12-27T23:51:39 Yeah! I'm curious, gitea and pagure show OBS commit history. Can people go to e.g. code.o.o make changes to a package there open MR, and if the MR gets accepted OBS syncs it? 2022-12-27T23:55:03 * Luciano[m] is seeing some additional things in gitea, e.g. repos have two branches, "devel" and "factory", while pagure doesn't. 2022-12-27T23:58:21 Hmm, pagure seems to track only devel projects instead. 2022-12-27T23:59:27 But I'm thinking it could be configured like gitea.