[libvirt-jenkins-ci PATCH] dco: build and publish a container image for DCO check

[Daniel P. Berrangé]

On Wed, Apr 08, 2020 at 04:21:31PM +0200, Andrea Bolognani wrote:
> On Wed, 2020-04-08 at 12:54 +0100, Daniel P. Berrangé wrote:
> > This builds a container image containing the DCO checking script.
> > It is then published at:
> > 
> >    registry.gitlab.com/libvirt/libvirt-jenkins-ci/check-dco:master
> > 
> > from where it can be used in all other project repositories. This
> > avoids having to copy the check-dco.py script into every repo
> > when we want to make changes to it.
> 
> So the idea is that the current dco job in, for example, libvirt,
> will become
> 
>   dco:
>     stage: prebuild
>     script:
>       - /check-dco.py
>     image: registry.gitlab.com/libvirt/libvirt-jenkins-ci/check-dco:master
> 
> and the same job will be added for all other projects, correct?

Yeah, exactly.

> 
> > +.build_container_template: &build_container_definition
> > +  image: docker:stable
> > +  stage: containers
> > +  services:
> > +    - docker:dind
> 
> TIL: dind means "Docker in Docker".
> 
> > +  before_script:
> > +    - docker info
> > +    - docker login registry.gitlab.com -u ${CI_REGISTRY_USER} -p ${CI_REGISTRY_PASSWORD}
> 
> For those following along at home, these variables are set
> automatically by GitLab:
> 
>   https://gitlab.com/help/ci/variables/predefined_variables.md
> 
> > +  script:
> > +    - docker build --tag ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG} -f ${NAME}/Dockerfile ${NAME}
> 
> The '-f ${NAME}/Dockerfile' part is unnecessary, because Dockerfile
> is the default name 'docker build' looks for and it will already
> enter the directory you provided (it wouldn't find the script
> otherwise).

Oh yes.

> > +    - docker push ${CI_REGISTRY_IMAGE}/${NAME}:${CI_COMMIT_REF_SLUG}
> 
> I'm not super happy about returning to the image:master naming
> convention, because most tooling around containers will expect the
> tag name to be 'latest' and we had just recently managed to adopt it
> throughout, but of course we need to make sure containers build off
> branches do not overwrite the known good image built from master...

We could have conditional logic that changes it to "latest" if
$CI_COMMIT_REF_SLUG == "master"

> I was going to suggest that, once the Jenkins stuff has been removed,
> we could rename the repository to lcitool, but maybe we should call
> it libvirt-ci instead? Or even just ci for that matter: we already
> have a number of non-prefixed repositories, and if they contain
> internal tools rather than projects that are intended to be released
> and distributed I think that's perfectly fine and if anything
> highlights the distinction.

Yeah, I was about to suggest that we just rename this to "libvirt-ci",
and accept the short term pain for people with broken URLs.

It is highly desirable to avoid very short, generic names like "ci",
because when you fork a repo into your own namespace, the repo names
must match and thus you risk a clash if you've forked a repo called
"ci" from an unrelated project.


> 
> > +++ b/check-dco/Dockerfile
> > @@ -0,0 +1,6 @@
> > +FROM centos:8
> > +
> > +RUN dnf -y install python3 git && \
> > +    dnf -y clean all
> > +
> > +COPY check-dco.py check-dco.py
> 
> I suggest using an absolute path for the destination for clarity,
> though it shouldn't actually make a difference.
> 
> Can we call the script "check-dco" without the .py suffix? The
> language used is an implementation detail that users will not be
> interested in.

Sure, we can drop the .py

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|