-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[e2e] Check labels documentation #1082
Conversation
The coordination.k8s.io/Lease used in e2e tests had an ownerReference, referencing non-existing node "kube-master" (non-existing in default Minikube deployments), so the lease was never created, and there was an (ignored) error during deletion. But, as a lease is automatically created for each node, there will always be leases in e2e, so there's no need to add one.
CI failures are actually good news:
It shows the following errors, that I will fix in a new commit:
/hold |
/assign @tariq1890 |
Added commit with documentation fixes; it resolves e2e test failures. |
This is so awesome. Thanks @olivierlemasle :)! |
func getLabelsDocumentation() (map[string][]string, error) { | ||
documentedMetrics := map[string][]string{} | ||
|
||
docPath := "../../docs/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any go libraries from stdlib
to get the directory the project root?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... I suppose we could use https://stackoverflow.com/a/38644571/625158, that uses runtime.Caller, but is it better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can go with that :). From the looks of it, I don't see any problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just tested adding:
_, b, _, _ = runtime.Caller(0)
Using make e2e
(from the root path of the project), b
is actually the absolute path of ./tests/e2e/main_test.go
. So I cannot get the documentation path with this method, without using ../../docs/
.
bd176d6
to
b6e0eab
Compare
b6e0eab
to
0387ebb
Compare
@tariq1890 PTAL thanks! |
/lgtm Thanks again @olivierlemasle :) |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: olivierlemasle, tariq1890 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What this PR does / why we need it:
This PR adds a new e2e test, checking that all metrics and labels found in
/metrics
endpoint are correctly documented (currently, metrics documentation is already checked withmake doccheck
, but labels are not validated).When part of the label name is documented in uppercase (e.g.
label_DAEMONSET_LABEL
), we will consider that by convention, it means that the uppercase part is dynamic (checked as a wildcard).NB: we also remove test manifest for
Lease
; as explained in the commit message, this was not necessary and caused an (ignored) error at the end of e2e tests.Which issue(s) this PR fixes:
Fixes #1078