On Mon, Feb 06, 2023 at 05:19:52PM +0100, Erik Skultety wrote: > On Mon, Feb 06, 2023 at 03:45:12PM +0000, Daniel P. Berrangé wrote: > > On Mon, Feb 06, 2023 at 02:52:57PM +0100, Erik Skultety wrote: > > > This is a follow up to: > > > https://listman.redhat.com/archives/libvir-list/2023-January/237201.html > > > > > > The effort here is to unify the way builds/tests are executed in GitLab CI vs > > > local container executions and make another step forward in terms of > > > reproducibility of (specifically) GitLab environments. > > > > > > Even though code to run all but one (coverity) jobs from GitLab via the > > > build.sh script is added with this series, local behavior remains the same as > > > before this series. The reason for that is that that will require more patches > > > ridding of the Makefile which is currently used and instead integrate usage of > > > lcitool with the ci/helper Python script which is currently the entry point for > > > local container executions. > > > > snip > > > > > .gitlab-ci.yml | 56 ++++++++++------------- > > > ci/Makefile | 16 ++++--- > > > ci/build.sh | 121 +++++++++++++++++++++++++++++++++++++++++++------ > > > ci/helper | 21 ++++++--- > > > 4 files changed, 155 insertions(+), 59 deletions(-) > > > mode change 100644 => 100755 ci/build.sh > > > > I'm really super unenthusiastic about this change, and also the similar > > change to add an ci/integration.sh. Shell scripting is something we > > worked hard to eliminate from libvirt. It is an awful language > > to do anything non-trivial with, error handling, lack of data > > structures, variable handling, portability are all bug generators. > > > > I know the .gitlab-ci.yml 'script' commands are technically shell > > script, but they are pretty trivial bits and don't have to worry > > about portability for dash vs bash etc, or complex control logic. > > The majority of it is just a simple list of commands to invoke, > > with the occasional conditional. > > > > The build.sh script is by contrast significantly more complex. By > > the very nature of taking "N" separate gitlab jobs and multiplexing > > them onto the one shell script, we're now having todo command line > > arg parsing in shell and de-multiplexing back to commands. The CI > > logic is now split up across even more sources - the gitlab config, > > the build.sh script and the meson.build files, which I think is > > worse for maintaining this too. > > True, except that even despite that, and I know what I'm talking about since I > wrote the integration jobs templates, GitLab is super picky; you can't debug > the syntax problems properly; certain common syntactic sugars don't work and > have to be replaced by less common counterparts; using YAML for bash formatting > leads to many table-flipping moments; you have to wait for Nx10 minutes before > it fails with some ridiculous error you would have caught early locally but you > couldn't because of the stage and artifact dependencies. So, once you have it > finally in place it's unarguably nice, but the road towards that isn't really > rainbow and unicorns either, it's still shell after all. Since I'm mainly > working on this part of libvirt, I myself would appreciate if I could easily > just run a single script instead of copy pasting commands one-by-one to a local > VM to catch stupid errors quickly rather than wait for GitLab to fail. > > > > > I appreciate the goal of being able to run CI commands locally, but > > I'm not feeling this approach is a net win. I'd much rather just > > having developers invoke the meson command directly, which is not > > that hard. > > Well, the big picture here is about making running integration tests locally > less painful than it is now...plus increase the amount of automation involved. > That's the major driver here - libvirt needs to be built locally in a safe > environment before integration tests could be run against it without touching > the host. So, with this script, while I agree with everything you said about > Bash, was just one step towards getting the automation I mentioned in place. I > also considered Python (yeah..why), but that would be super akward. > > TL;DR: if we don't do anything about how we currently maintain the build > recipes (note we're maintaining 2 already), everybody will continue ignoring the > integration test suite, unless we enable merge requests where the status quo > would be somewhat (but not completely) eliminated. With integration tests you > can't ignore the aspect of getting feedback early compared to waiting for > GitLab CI. > > > > > If we do really want to provide something that 100% mirrors the > > gitlab CI job commands though, I'm even more convinced now that > > we should be using the .gitlab-ci.yml as the canonical source. > > > > Since I last mentioned that idea, I found out that this should > > be something we can achieve via the gitlab runner 'exec' command. > > Haven't heard of ^this one, but I wonder how we can get something meaningful > out of it for the purposes I mentioned above. > > Erik So, I had a brief look at gihttps://gitlab.com/gitlab-org/gitlab-runner/-/issues/2797tlab-runner exec this morning. Long story short, it's deprecated [1] and though it was scheduled for complete removal, that plan was put on hold for now. Nevertheless, it's not getting any features that are introduced to .gitlab-ci.yml. The gist is that it doesn't understand 'include' nor 'extends' which we're using heavily across our gitlab configuration hierarchy, so it's a no-go. It also doesn't support artifacts in any form, so while it technically should be possible to save RPM builds in a volume (not sure if the bind mount is cleared on job completion or not) we could not pass them into a VM running the integration tests the same convenient way as we do in the CI. Another thing to consider is that this solution also requires to have the gitlab-runner binary installed locally, which many in the community may not be comfortable with, but that's up for a debate, but hey, you can always build it yourself. There's a plan to rework GitLab's local CI execution functionality heavily [2] before the 'exec' command is sunset, but ultimately their plan is have something working by Q3 FY24, which can be anywhere from March 2024 until September 2024, that's more than a year ahead. In your reply you mentioned the unappealing complexity of the script potentially leading to bugs. At the same time though one currently can't consume .gitlab-ci.yml recipes to run a local integration test in a VM. So, how about I get rid of the multiplexing and CLI parsing by placing each job recipe in a standalone script or even going one step further and extract the commonalities to a separate file that would be source from within each job script? Would you consider that meeting each other halfway? As for POSIX compliance, I guess this would be a soft-requirement based on whether shellcheck was run during review, does gitlab do something better? IIUC there's no pre-check and you'll only know after a job was executed that there was an incompatible statement. [1] https://docs.gitlab.com/runner/commands/#gitlab-runner-exec-deprecated [2] https://gitlab.com/gitlab-org/gitlab-runner/-/issues/2797 Regards, Erik
