-
Notifications
You must be signed in to change notification settings - Fork 494
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 featuregate for k8s 1.28 native sidecar container #2801
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: enhancement | ||
|
||
# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) | ||
component: pkg/sidecar | ||
|
||
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
note: Add native sidecar injection behind a feature gate which is disabled by default. | ||
|
||
# One or more tracking issues related to the change | ||
issues: [2376] | ||
|
||
# (Optional) One or more lines of additional information to render under the primary note. | ||
# These lines will be padded with 2 spaces and then inserted directly into the document. | ||
# Use pipe (|) for multiline entries. | ||
subtext: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please document here the feature gate as a flag e.g. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,18 @@ const ( | |
) | ||
|
||
var ( | ||
// EnableNativeSidecarContainers is the feature gate that controls whether a | ||
// sidecar should be injected as a native sidecar or the classic way. | ||
// Native sidecar containers have been available since kubernetes v1.28 in | ||
// alpha and v1.29 in beta. | ||
// It needs to be enabled with +featureGate=SidecarContainers. | ||
// See: | ||
// https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/#feature-gates-for-alpha-or-beta-features | ||
EnableNativeSidecarContainers = featuregate.GlobalRegistry().MustRegister( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we could on k8s 1.28+ always use the native sidecar pattern and keep using the existing approach on older k8s versions. This way no feature flag is needed and we would always use a recommended approach. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can do that, but we would expect different results in our e2e tests. Which makes it hart to execute the same tests on both platform versions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm ok with that as well. I can't think of any issues it might cause. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, then I will enable it for the 1.28+ e2e tests. Would be nice it it makes it into v0.111.0. |
||
"operator.sidecarcontainers.native", | ||
featuregate.StageAlpha, | ||
featuregate.WithRegisterDescription("controls whether the operator supports sidecar containers as init containers"), | ||
frzifus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
// PrometheusOperatorIsAvailable is the feature gate that enables features associated to the Prometheus Operator. | ||
PrometheusOperatorIsAvailable = featuregate.GlobalRegistry().MustRegister( | ||
"operator.observability.prometheus", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
--- | ||
apiVersion: v1 | ||
kind: Pod | ||
metadata: | ||
annotations: | ||
sidecar.opentelemetry.io/inject: "true" | ||
name: myapp | ||
spec: | ||
containers: | ||
- name: myapp | ||
initContainers: | ||
- name: otc-container | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we verify that this has the correct restart policy? |
||
status: | ||
containerStatuses: | ||
- name: myapp | ||
ready: true | ||
started: true | ||
initContainerStatuses: | ||
- name: otc-container | ||
ready: true | ||
started: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
--- | ||
apiVersion: opentelemetry.io/v1beta1 | ||
kind: OpenTelemetryCollector | ||
metadata: | ||
name: a-sidecar | ||
spec: | ||
mode: sidecar | ||
resources: | ||
limits: | ||
cpu: 500m | ||
memory: 128Mi | ||
requests: | ||
cpu: 5m | ||
memory: 64Mi | ||
|
||
config: | ||
receivers: | ||
otlp: | ||
protocols: | ||
http: {} | ||
exporters: | ||
debug: {} | ||
service: | ||
pipelines: | ||
metrics: | ||
receivers: [otlp] | ||
exporters: [debug] | ||
--- | ||
apiVersion: v1 | ||
kind: Pod | ||
metadata: | ||
name: myapp | ||
annotations: | ||
sidecar.opentelemetry.io/inject: "true" | ||
spec: | ||
containers: | ||
- name: myapp | ||
image: jaegertracing/vertx-create-span:operator-e2e-tests | ||
frzifus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ports: | ||
- containerPort: 8080 | ||
protocol: TCP |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json | ||
apiVersion: chainsaw.kyverno.io/v1alpha1 | ||
kind: Test | ||
metadata: | ||
creationTimestamp: null | ||
name: native-sidecar | ||
spec: | ||
steps: | ||
- name: step-00 | ||
try: | ||
- apply: | ||
file: 00-install.yaml | ||
- assert: | ||
file: 00-assert.yaml |
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.
collector