Page MenuHomePhabricator

Add CI checks for golang admission controllers
Closed, ResolvedPublic

Description

Per conversation with @hashar, this is a ticket to create a CI environment for the two Go repos we've made for the new Toolforge Kubernetes deployment.

The requirement is go 1.11+ (for modules) and to run two commands as checks:
go test ./… and golint ./… which will basically lint and run the tests that are defined wherever they are in the repos.

The repos in question are:

  • labs/tools/registry-admission-webhook
  • cloud/toolforge/ingress-admission-controller

Event Timeline

Bstorm removed Bstorm as the assignee of this task.Oct 22 2019, 7:13 PM
Bstorm triaged this task as Medium priority.
Bstorm created this task.

I have bring this up to Release-Engineering-Team team meeting.

We do not currently have golang based containers in integration/config.git , though it is probably not too complicated to set one up.

I think containers for that is probably fine. Not everything has to jump over to the service pattern immediately, especially stand-alone things.

We do have a docker-pkg managed image for Go projects, docker-registry.wikimedia.org/golang:1.11.5-1. It's currently being utilized as a base image by Blubber, and it can likely be used here.

For the CI side, we can either define jobs directly in JJB for golint and go test, or utilize the deployment pipeline. The latter is quite flexible, even for test-only use cases that don't result in published images or deployed containers. I'm available to pair with @Bstorm on pipeline configuration if that's the right route for these projects.

Whichever one you folks want to use is alright by me. I don't know much about the deployment pipeline. These get deployed to a Cloud VPS-based Kubernetes cluster via the Toolforge docker registry, if that affects anything here. My big concern is getting tests run when reviewing a commit.

@Jdforrester-WMF 👋🏻 This is the ticket I mentioned at TechConf

I'll cookie-lick this one. ;-)

Do you have a repo you'd like to use this on now?

Change 552865 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] [WIP] jjb: Add a golang jjb defintion for golint and go test

https://gerrit.wikimedia.org/r/552865

@Jdforrester-WMF Yup, these are the two repos. They both need the exact same config ideally.

labs/tools/registry-admission-webhook
cloud/toolforge/ingress-admission-controller
Jdforrester-WMF lowered the priority of this task from Medium to Low.Jan 8 2020, 6:30 PM

Sorry, this ran into the issue of me having to learn from scratch the entire go ecosystem and set of testing tools.

On reflection, I think it'd be better if instead we set you up with pipelinelib (where CI config lives inside each repo, rather than me building a generic set of tests that work for everyone's code); @dduvall is the expert writing the documentation for how to do this and has said he'd be OK to pair with you on getting it work. Would that be OK?

Change 552865 abandoned by Jforrester:
[WIP] jjb: Add a golang jjb defintion for golint and go test

Reason:
This didn't work enough to justify.

https://gerrit.wikimedia.org/r/552865

@Bstorm, we've been toiling quite a bit on documentation for Blubber and PipelineLib, and I've just added a Go-specific user guide that should get you what you need for these two projects.

If you don't mind being a guinea pig for the documentation, have at it! Of course, feel free to ping me on IRC (marxarelli) and/or hit me up with an invite for pairing as well. I'd love to help out.

Change 577603 had a related patch set uploaded (by Bstorm; owner: Bstorm):
[cloud/toolforge/ingress-admission-controller@master] ci: introduce a pipeline config

https://gerrit.wikimedia.org/r/577603

Change 577626 had a related patch set uploaded (by Bstorm; owner: Bstorm):
[integration/config@master] jjb: test pipeline for cloud/toolforge/ingress-admission-controller

https://gerrit.wikimedia.org/r/577626

Bstorm added a comment.Mar 6 2020, 5:38 PM

@dduvall Presuming my patches are at all correct, I can say that the documentation wants more of an outline or overview of the process in the beginning for those of us who rarely have touched the CI stuff. I'm thinking something like an outline of the steps in the beginning before the tutorial portion so that people who read in dreadful ways like myself don't end up jumping ahead and backtracking :)

More importantly, I think there are maybe typos and bits that need more specificity here https://wikitech.wikimedia.org/wiki/PipelineLib/Guides/How_to_configure_CI_for_your_project. This seems to have a typo about which file to edit https://wikitech.wikimedia.org/wiki/PipelineLib/Guides/How_to_configure_CI_for_your_project#Tell_Zuul_to_invoke_your_pipeline_jobs_to_test_project_patchsets, and I am left wondering if I am using the right names, for instance. Maybe be very clear what the name under projects in the Zuul config will mean, and that you must fill in the variables from your project-pipelines.yaml instead of pasting in the trigger bit, if I figured things out correctly in my patches at least. :)

Bstorm added a comment.Mar 6 2020, 5:50 PM

The docs definitely got me further than I was, though!

Bstorm added a comment.Mar 6 2020, 6:09 PM

Interestingly, I got this from CI on my change to integration-config:

17:19:38 ======================================================================
17:19:38 FAIL: test_projects_have_pipeline_gate_and_submit (tests.test_zuul_scheduler.TestZuulScheduler)
17:19:38 ----------------------------------------------------------------------
17:19:38 Traceback (most recent call last):
17:19:38   File "/src/tests/test_zuul_scheduler.py", line 362, in test_projects_have_pipeline_gate_and_submit
17:19:38     self.assertEqual([], lacks_gate)
17:19:38 AssertionError: Lists differ: [] != ['toolforge-ingress-admission-...
17:19:38 
17:19:38 Second list contains 1 additional elements.
17:19:38 First extra element 0:
17:19:38 'toolforge-ingress-admission-controller'
17:19:38 
17:19:38 - []
17:19:38 + ['toolforge-ingress-admission-controller']
Bstorm added a comment.EditedMar 6 2020, 6:15 PM

I added:

gate-and-submit:
  - noop

And CI seems happy.

hashar added a comment.Mar 6 2020, 6:22 PM

What is written on the can: test_projects_have_pipeline_gate_and_submit

All projects must have a job in gate-and-submit, that is to ensure there is no regression and no conflicts when several changes are +2ed together :] Usually one can just reuse the same set of jobs that are used in the test pipeline :]

Change 577626 merged by jenkins-bot:
[integration/config@master] jjb: test pipeline for cloud/toolforge/ingress-admission-controller

https://gerrit.wikimedia.org/r/577626

Change 577603 merged by Bstorm:
[cloud/toolforge/ingress-admission-controller@master] ci: introduce a pipeline config

https://gerrit.wikimedia.org/r/577603

Change 577651 had a related patch set uploaded (by Bstorm; owner: Bstorm):
[labs/tools/registry-admission-webhook@master] pipeline: add config for the pipeline

https://gerrit.wikimedia.org/r/577651

Change 577653 had a related patch set uploaded (by Bstorm; owner: Bstorm):
[integration/config@master] jjb: test pipeline for labs/tools/registry-admission-webhook

https://gerrit.wikimedia.org/r/577653

Change 577653 merged by jenkins-bot:
[integration/config@master] jjb: test pipeline for labs/tools/registry-admission-webhook

https://gerrit.wikimedia.org/r/577653

Change 577651 merged by Bstorm:
[labs/tools/registry-admission-webhook@master] pipeline: add config for the pipeline

https://gerrit.wikimedia.org/r/577651

Bstorm closed this task as Resolved.Mar 6 2020, 8:06 PM

It all works. Thanks!