1. automatically add etcd to current PATH when calling kube::etcd::install
2. call kube::etcd::install from update-openapi-spec
3. don't call kube::golang::setup_env twice
Some code owners might want this for specific packages, like cmd/kubeadm.
This cannot be enabled for everything because:
- a lot of existing code doesn't pass (-> can't be in base config)
- a lot of packages don't need it (-> shouldn't even be a hint)
The in-tree configs use a relative path to find logcheck.so. This is useful
because then the invocation of golangci-lint also works outside of the script.
But when running with a containerized build, GOBIN points somewhere else. For
that case, a temporary copy of the configuration has to be created with an
absolute path.
Contributors have created PRs because the linter found them. That is often not
necessary and should be discussed before working on a PR.
While at it, output formatting gets updated a bit (extra blank lines, wording).
If something goes wrong during the test registration phase, the only solution
so far was to panic. This is not user-friendly and only allows to report one
problem at a time.
If initialization can continue, then a better solution is to record a bug,
continue, and then report all bugs together.
This also works when just listing tests. The new verify-e2e-suites.sh uses that
to check all test suites (identified as "packages that call
framework.AfterReadingAllFlags", with some exceptions) as part of
pull-kubernetes-verify.
Example output for a fake
framework.RecordBug(framework.NewBug("fake bug during SIGDescribe", 0))
in test/e2e/storage/volume_metrics.go:
```
$ hack/verify-e2e-suites.sh
go version go1.21.1 linux/amd64
ERROR: E2E test suite invocation failed for test/e2e.
ERROR: E2E suite initialization was faulty, these errors must be fixed:
ERROR: test/e2e/storage/volume_metrics.go:49: fake bug during SIGDescribe
E2E suite test/e2e_kubeadm passed.
E2E suite test/e2e_node passed.
```
Instead of invoking verify-golangci-lint.sh directly from Prow jobs,
those Prow jobs should use "make verify WHAT=...". The advantage is
that the common code for running verify targets will be used, which
includes producing JUnit files.
Providing simple wrappers for strict linting of PRs (=
verify-golangci-lint-pr.sh) and event stricter linting of PRs with hints
enabled (= verify-golangci-lint-pr-hints.sh) enables those WHAT targets.
The spaces are redundant because Ginkgo will add them itself when concatenating
the different test name components. Upcoming change in the framework will
enforce that there are no such redundant spaces.
Using StartRecordingToSinkWithContext instead of StartRecordingToSink and
StartLogging instead of StartStructuredLogging has several advantages:
- Spawned goroutines no longer get stuck for extended periods of
time during shutdown when passing in a context that gets canceled.
- Log output can be directed towards a specific logger instead of the global
default, for example one which writes to a testing.T instance.
- The new methods return an error when something went wrong instead of
merely recording the error.
That last point is the reason for deprecating the old methods instead of merely
adding new alternatives.
Setting a context when constructing an EventBroadcaster makes calling Shutdown
optional. It can also be used to specify the logger.
Both EventRecorder interfaces in tools/events and tools/record now have a
WithLogger helper. Using that method is optional, but recommended to support
contextual logging properly. Without it, errors that occur while emitting an
event are not associated with the caller.
for now:
- shim FORCE_HOST_GO to GOTOOLCHAIN=local
- treat GOTOOLCHAIN set and !=auto like FORCE_HOST_GO
- otherwise set GOTOOLCHAIN=go${GO_VERSION} and fallback to gimme if necessary
TODO: set toolchain statements in go.mod files and keep them in sync
This particualar warning didn't make it into
https://github.com/kubernetes/kubernetes/issues/117288. Discussion on Slack
concluded that "it's hard to have a universal policy for all functions marked
deprecated" and thus this can only be a hint which must be considered on a
case-by-case basis.
For example, APIs like sets.String are very unlikely to ever go away, therefore
it is entirely up to developers whether they switch to sets.Set even though
sets.String is marked as deprecated.
Ideally, the deprecation message should explain this. It doesn't for sets ("use
generic Set instead"), so a better message in that case would have been
"consider using generic Set instead".
The voting in https://github.com/kubernetes/kubernetes/issues/117288 led to
one check that got rejected ("ifElseChain: rewrite if-else to switch
statement") and several that are "nice to know".
golangci-lint's support for issue "severity" is too limited to identify "nice
to know" issues in the output (filtering is only by linter without considering
the issue text; not part of text output). Therefore a third configuration gets
added which emits all issues (must fix and nits). The intention is to use
the "strict" configuration in pull-kubernetes-verify and the "hints"
configuration in a new non-blocking pull-kubernetes-linter-hints.
That way, "must fix" issues will block merging while issues that may be useful
will show up in a failed optional job. However, that job then also contains
"must fix" issues, partly because filtering out those would make the
configuration a lot larger and is likely to be unreliably (all "must fix"
issues would need to be identified and listed), partly because it may be useful
to have all issues in one place.
The previous approach of manually keeping two configs in sync with special
comments didn't scale to three configs. Now a single golangci.yaml.in with
text/template constructs contains the source for all three configs. A new
simple CLI frontend for text/template (cmd/gotemplate) is used by
hack/update-golangci-lint-config.sh to generate the three flavors.
Several verify scripts used the same pattern of "check for clean working tree,
generated files, check for diffs". The code for that is now in
kube::verify::generated, defined in hack/lib/verify-generated.sh, and those
scripts just source that.
For some reason, in go1.21, go list does not allow
importing main packages anymore, even if it is for
the sake of tracking dependencies (which is a valid
use case).
A suggestion to work around this is to use -e flag to
permit processing of erroneous packages. However, this
doesn't seem prudent.
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
That release is the first one with official support for Go 1.21. go-ruleguard
must be >= 0.3.20 because of
https://github.com/quasilyte/go-ruleguard/issues/449 with Go
1.21. golangci-lint itself doesn't depend on a recent enough release yet, so
this was done manually.
Client-side extract calls depend on `managedFields`, which might not be
available. Therefore they should not be used in production code.
They are okay in test files (because the API has to be tested), in the
generated code (because the various type specific APIs still need to be
provided) and in unstructured.go (same reason).
This bump is done since the latest version of staticcheck includes
a fix for a false positive reported by us, discovered while bumping
to go1.20
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
For example, this is a false positive that currently exists in the code base:
test/e2e_node/dra_test.go:129:4: ginkgo-linter: use a function call in Consistently. This actually checks nothing, because Consistently receives the function returned value, instead of function itself, and this value is never changed; consider using `gomega.Consistently(ctx, e2epod.Get).WithArguments(f.ClientSet, pod).WithTimeout(podInPendingStateTimeout).Should(e2epod.BeInPhase(v1.PodPending),
"Pod should be in Pending state as resource preparation time outed")` instead (ginkgolinter)
gomega.Consistently(ctx, e2epod.Get(f.ClientSet, pod)).WithTimeout(podInPendingStateTimeout).Should(e2epod.BeInPhase(v1.PodPending),
^
It's a false positive because e2epod.Get returns the function that Consistently
is meant to call.
This could be worked around by assigning e2epod.Get(f.ClientSet, pod) to a
variable and then use that variable, but that is less readable.
In the wait_node_ready function, two steps are performed:
1.Check if the node exists
2.Wait for the node to enter the ready state
If one step fails, the second step should not continue, wasting 300 seconds.
Container runtimes like CRI-O and containerd reuse the code by copying
it from Kubernetes. To have a single source of truth for the streaming
server we now move the already isolated implementation to the
k8s.io/kubelet staging repository. This way runtimes can re-use the code
without copying the parts.
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
Because this doesn't get invoked through verify.sh, we have to call
logJu ourselves to get a JUnit file. errexit must not be set when
calling logJu, otherwise it does no post-processing.
When sh2ju.sh was called to generate the junit_verify.xml, it used to include
the entire output of a failed script twice: once as failure message, once as
log output.
This output can be large and often the actual failure isn't near the top, but
rather at the end or (in the case of the different golangci-lint invocations)
embedded in the log. This makes them hard to see at a glance when looking at
the Prow result page for a job.
Now a verify script can prefix relevant lines with "ERROR: " and then only
those lines are used as failure message in JUnit, without that prefix.
That string was chosen because Prow itself also then picks up those lines when
viewing the entire build log and it is unlikely that some script prints such
lines when they are not meant to be part of the failure.
If some script outputs no such lines, "see stderr for details" is used as
failure message. This is better than before because it avoids the redundancy.
Fixed ginkgo warning
You're using deprecated Ginkgo functionality:
=============================================
--untilItFails is deprecated, use --until-it-fails instead
Used consistent approach with this flag in e2e_node and e2e scripts.
