simple flakefinder implementation and job configuration #146
Conversation
|
This first version is working already, although a lot of cleanup is needed. |
|
I'm trying to get the bazel build working, running into compile errors that I do not understand: Using @slintes can you help me understand what is the problem here? |
| github.com/onsi/ginkgo v1.7.0 // indirect | ||
| github.com/onsi/gomega v1.4.3 // indirect | ||
| github.com/sirupsen/logrus v1.4.2 | ||
| google.golang.org/api v0.5.0 |
There was a problem hiding this comment.
I believe I have seen this issue before. The issue was related to the version mismatch.
Since we have switched to golang modules, it might happen.
I would propose to add
google.golang.org/grpc v1.19.0
here. And see whether it helps. It should help go mod to figure out the minimal version required.
There was a problem hiding this comment.
Also please make sure all bazel caches are clean.
There was a problem hiding this comment.
Thanks for your help, @petrkotas!
I suppose I just should add the line in go.mod, right?
I understood from reading the go modules manual that you would normally do a go get google.golang.org/grpc@v1.19.0, but that produced a lot of other entries? However, will try out and tell how it goes. Thanks again!
There was a problem hiding this comment.
not sure, maybe it has to go into replaces section, since this is not required directly by us, but should be overridden in our dependencies IIUC?
There was a problem hiding this comment.
AFAIK when you do not include the direct version, go mod will scan through the code dependencies, it will find there is grpc required, will select minimum usable version and will include it.
If you include the minimum version yourself, you are forcing go mod to use the one you require.
Yes just place it somewhere in between the commented lines.
There was a problem hiding this comment.
Tried it, didn't help.
I got feedback from the bazel mailing list pointing me towards rules_go - Avoiding conflicts (proto related paragraph) and this related GitHub issue. Am still trying to understand and integrate this, could still need some help.
There was a problem hiding this comment.
Just for reference, the solution has been described here.
| @@ -13,7 +13,7 @@ go_library( | |||
| deps = [ | |||
| "//vendor/golang.org/x/oauth2:go_default_library", | |||
| "//vendor/golang.org/x/oauth2/google:go_default_library", | |||
| "//vendor/google.golang.org/grpc/naming:go_default_library", | |||
| "@org_golang_google_grpc//naming:go_default_library", | |||
There was a problem hiding this comment.
tbh, I don't think editing files in the vendor dir is a good idea. I expect that it will be overwritten sooner or later, e.g. when running go mod vendor?
There was a problem hiding this comment.
I thought so too but that was the only thing that helped.
IIUC the correct solution to this would be to make use of the go_proto_library rules? Do you have an idea on how to do this? I couldn't figure this out myself...
There was a problem hiding this comment.
You are right. Actually the combination of
go mod vendor
bazel run //:gazelle
overrides the changes in vendor/google.golang.org/api/internal/BUILD.bazel again :(
There was a problem hiding this comment.
I still have no quick idea, will try to find some time to have look tomorrow
There was a problem hiding this comment.
I'm currently reading up on gazelle dependency resolution, trying out the # gazelle resolve directive might be a way.
There was a problem hiding this comment.
Looks like this fixed it. Will need to read up why gazelle added some crlf to the yaml.
There was a problem hiding this comment.
Noticed it's not gazelle, the file is retrieved like this when using go mod vendor, so leaving it like this for the moment.
There was a problem hiding this comment.
AFAIU this is a file that is needed for appveyor under Windows, AFAIR the crlf is default there, so I guess this is fine.
b8c7958 to
da43481
Compare
|
/test periodic-publish-flakefinder-reports |
1 similar comment
|
/test periodic-publish-flakefinder-reports |
|
/retest |
1 similar comment
|
/retest |
|
/test periodic-publish-flakefinder-reports |
2 similar comments
|
/test periodic-publish-flakefinder-reports |
|
/test periodic-publish-flakefinder-reports |
bazel go module support is not yet that great
Write report to kubevirt bucket under /reports/flakefinder/flakefinder-$isodate.html, then create index.html as wrapping page. This page contains the links to the last 50 reports.
NOTE: this is not yet working, `bazel build //robots/flakefinder` yields a compile error that I need support with to understand and fix.
|
I just noticed that the complete set has not been implemented. This PR currently only supports creating weekly reports. I think we should first go with the weekly report which is ready now, then I will work on creating the other ones. Opinions? |
|
In regards to my comment above I opened #168 to further work on this. |
| </head> | ||
| <body> | ||
|
|
||
| <table> |
There was a problem hiding this comment.
could you format that HTML? It's quite hard to read.
robots/flakefinder/downloader.go
Outdated
| @@ -0,0 +1,197 @@ | |||
| /* | |||
| Copyright 2018 The Kubernetes Authors. | |||
There was a problem hiding this comment.
The Kubernetes Authors? Is this code a copy from some k8s repo? If not then it should be Kubevirt :)
EDIT: or This file is part of the KubeVirt project
|
So far the PR looks good to me, I didn't finish the detailed review (I will finish it on Monday). Do you mind to add unit tests for flakefinder? I think it would be useful. |
|
@mfranczy I totally agree that I should have written tests. I'm doing this in the new PR to extend the functionality as planned. Maybe we can live with this one it as it is now? |
|
@dhiller I am okay with your plan. |
|
|
||
| #### Run the job locally using [`phaino`](https://github.com/kubernetes/test-infra/tree/master/prow/cmd/phaino) | ||
|
|
||
| phaino --privileged /tmp/prowjob.yaml |
There was a problem hiding this comment.
let's use generic examples, please replace /home/dhiller by $HOME
There was a problem hiding this comment.
Oops, good point...
| merged time.Duration | ||
| } | ||
|
|
||
| type client interface { |
There was a problem hiding this comment.
could you format that interface that methods have the same indents number?
There was a problem hiding this comment.
Ah interesting, looks like a tabs vs. spaces issue. Will correct this.
- Fix formatting - remove personal path
|
/lgtm |
FTR the further work has been done in #168 which is feature complete right now. I'm working on cleaning up right now. |
|
Just noticed one issue: the 24h report is empty, will have a look. |
|
Another one is that the monthly report has very few entries. |
|
The reason for the 24h report not having any entries is that the junit xml flie can't be found. |
|
superseded by #168 /close |
|
@slintes: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
* hack: add simple pre-flight check to cluster-up It still happens nowdays that we run kubevirtci on wrongly provisioned boxes with incorrect kvm setup. This patch adds a simple pre-flight check script to catch the most common issues, hopefully preventing more time wasted in debugging cluster running on these wrong setups. Signed-off-by: Francesco Romani <fromani@redhat.com> * pre-flight check: address reviewer comments Signed-off-by: Francesco Romani <fromani@redhat.com>
Note: Work originally started by rmohr
Closes #142
flakefinder
flakefinder does the following:
/reports/flakefinder/flakefinder-$date.html- extract skipped/failed/success from the junit results and create a html tableindex.htmlOutput is published and visible here:
https://storage.googleapis.com/kubevirt-prow/reports/flakefinder/index.html
Selecting the right builds:
job configuration
Job is currently configured to be run every week. Configuration was put in
github/ci/prow/files/jobs/kubevirt/kubevirt-periodics.yaml/robots/flakefinder/BUILD.bazel
The build file for flakefinder now conains steps to create and push a docker image to the registry