-
Notifications
You must be signed in to change notification settings - Fork 28
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
add allowPrivilegedContainers to flavors #287
Conversation
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.
Do we maybe want to add it as metadata field so that we can see the differences in the execution?
And we could also think of adding it to our dimensions to reflect it in the dashboard/alerting?
WDYT?
https://github.com/gardener/test-infra/blob/master/pkg/testmachinery/metadata/types.go#L18
Regarding metadata, I though it would be enough to distinguish via flavor description, but you are right, a dedicated metadata is better, I'll add that, thanks for the suggestion. |
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.
just a minor nit rest is lgtm
23ddbe9
to
3edd57f
Compare
da4dfbe
to
10c7f49
Compare
added metadata support, hope I didn't miss some places, ptal |
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.
we also need to read the metadta from the testrun here https://github.com/gardener/test-infra/blob/master/pkg/testmachinery/metadata/metadata.go#L57
How about adding it also here https://github.com/gardener/test-infra/blob/master/pkg/testmachinery/metadata/metadata.go#L41 so that it will also be displayed in the tm dashboard and the slack notifications?
10c7f49
to
8bc7e00
Compare
Added.
I actually envisioned to put it in the description of the flavor configuration, but sure, putting it directly in the code is more consistent in the end, so added it. PTAL |
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.
just one minor nit. rest is lgtm
8bc7e00
to
2c3492b
Compare
What this PR does / why we need it:
Adds
allowPrivilegedContainers
as an additional option in the flavor configuration. Gardener shoots default totrue
is this option is not set, hence you can use it to disable privileged containers for certain tests or have your tests run against both: shoots withallowPrivilegedContainers="true"
and shoots withallowPrivilegedContainers="false"
Release note: