Fixes golint in test/utils/image #69598
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change adds comments to exported things and renames the tcp, http, and exec probe interfaces to just be Prober within their namespace. Issue kubernetes#68026
…sers who are not owners.
If the permissions are not setup correctly the example fails. Change-Id: I167ef68be66f8b56740236ae475c3b7fdcc0dfb5
The default version in kubeadm is now `stable-1`. This will pull a version from the `stable-1.txt` endpoint which might end up being newer than the version of the client by a magnitude of MINOR or even a MAJOR release. To be able to prevent this scenario add the new helper function: validateStableVersion() This function determines if the remote version is newer than the local client version and if that's the case it returns `stable-X.xx` that conforms with the version of the client. If not it returns the remote version.
Storing settings in the framework's TestContext is not something that out-of-tree test authors can do because for them the framework is a read-only upstream component. Conceptually the same is true for in-tree tests, so the recommended approach is to define configuration settings in the code that uses them. How to do that is a bit uncertain. Viper has several drawbacks (maintenance status uncertain, cannot list supported options, cannot validate the configuration file). How to handle configuration files is currently getting discussed for kubeadm, with similar concerns about Viper (kubernetes/kubeadm#1040). Instead of making a choice now for E2E, the recommendation is that test authors continue to define command line flags as before, except that they should do it in their own code and with better flag names. But the ability to read options also from a file is useful, so several enhancements get added: - all settings defined via flags can also be read from a configuration file, without extra work for test authors - framework/config makes it possible to populate a struct directly and define flags with a single function call - a path and file suffix can be given to --viper-config (as in "--viper-config /tmp/e2e.json") instead of expecting the file in the current directory; as before, just plain "--viper-config e2e" still works - if "--viper-config" is set, the file must exist; otherwise the "e2e" config is optional (as before) - errors from Viper are no longer silently ignored, so syntax errors are detected early - Viper support is optional: test suite authors who don't want it are not forced to use it by the e2e/framework
Tests shouldn't have to use the central context for their settings,
because conceptually tests and framework get developed independently.
This does not yet use the new framework/config utility code because
that code still needs to be reviewed.
Besides moving the flags, they also get renamed from the top-level
"--csiImage{Version|Registry}" to
"--storage.csi.image.{version|registry}". These flags were introduced
fairly recently and shouldn't be in use much, so now is a good time to
introduce a hierarchical naming for storage flags, in particular
because more flags will be added soon.
Tests settings should be defined in the test source code itself because conceptually the framework is a separate entity that not all test authors can modify. Using the new framework/config code also has several advantages: - defaults can be set with less code - no confusion around what's a duration - the options can also be set via command line flags While at it, a minor bug gets fixed: - readConfig() returns only defaults when called while registering Ginkgo tests because Viperize() gets called later, so the scale in the logging soak test couldn't really be configured; now the value is read when the test runs and thus can be changed The options get moved into the "instrumentation.logging" resp. "instrumentation.monitoring" group to make it more obvious where they are used. This is a breaking change, but that was already necessary to improve the duration setting from plain integer to a proper time duration.
Tests settings should be defined in the test source code itself because conceptually the framework is a separate entity that not all test authors can modify. For the sake of backwards compatibility the name of the command line flags are not changed.
Generated via hack/update-bazel.sh.
fix logging flushing issue in GetVolumeLimits
Fixed go format and unit test. Collapse lines. Switched to using regional throughout and added warning for HA Zonal.
The message right before the kubelet boots up the control plane is misleading because the image pulling is now part of preflight. Remove the message.
This change fixes the test/utils/image package golint errors.
k8s-ci-robot
added
release-note-none
k8s-ci-robot
removed
the
lgtm
|
New changes are detected. LGTM label has been removed. |
k8s-ci-robot
added
size/XXL
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. I understand the commands that are listed here. |
k8s-ci-robot
added
cncf-cla: no
|
oh... yeah I think I messed up this PR |
k8s-ci-robot
added
area/apiserver
area/code-generation
area/kubeadm
area/kubectl
area/kubelet
sig/api-machinery
|
@jonfriesen: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This change fixes the test/utils/image package golint errors.
Which issue(s) this PR fixes:
refer #68026
Special notes for your reviewer:
I renamed
ImageConfigtoConfigto fix stuttering (eg.image.ImageConfig). I couldn't find a spot where this would break other packages. Though I'm not exactly sure what the best practice is on renaming a public struct. Guidance would be appreciated :)Release note: