Skip to content
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

Not preserve the ownership of the files copied in the autoinstrumentation #2695

Merged
merged 14 commits into from
Mar 13, 2024
Merged
16 changes: 16 additions & 0 deletions .chloggen/fix-2655.yaml
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: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. operator, target allocator, github action)
component: instrumentation

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Don't preserve ownership of files copied from the autoinstrumenation image. This avoids issues when instrumenting workloads running as non-root"

# One or more tracking issues related to the change
issues: [2655]

# (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:
2 changes: 1 addition & 1 deletion pkg/instrumentation/dotnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func injectDotNetSDK(dotNetSpec v1alpha1.DotNet, pod corev1.Pod, index int, runt
pod.Spec.InitContainers = append(pod.Spec.InitContainers, corev1.Container{
Name: dotnetInitContainerName,
Image: dotNetSpec.Image,
Command: []string{"cp", "-a", "/autoinstrumentation/.", dotnetInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", dotnetInstrMountPath},
Resources: dotNetSpec.Resources,
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
Expand Down
8 changes: 4 additions & 4 deletions pkg/instrumentation/dotnet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func TestInjectDotNetSDK(t *testing.T) {
{
Name: "opentelemetry-auto-instrumentation-dotnet",
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-dotnet"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-dotnet"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-dotnet",
MountPath: "/otel-auto-instrumentation-dotnet",
Expand Down Expand Up @@ -165,7 +165,7 @@ func TestInjectDotNetSDK(t *testing.T) {
{
Name: "opentelemetry-auto-instrumentation-dotnet",
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-dotnet"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-dotnet"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-dotnet",
MountPath: "/otel-auto-instrumentation-dotnet",
Expand Down Expand Up @@ -394,7 +394,7 @@ func TestInjectDotNetSDK(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-dotnet"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-dotnet"},
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: "/otel-auto-instrumentation-dotnet",
Expand Down Expand Up @@ -473,7 +473,7 @@ func TestInjectDotNetSDK(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-dotnet"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-dotnet"},
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: "/otel-auto-instrumentation-dotnet",
Expand Down
2 changes: 1 addition & 1 deletion pkg/instrumentation/nodejs.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func injectNodeJSSDK(nodeJSSpec v1alpha1.NodeJS, pod corev1.Pod, index int) (cor
pod.Spec.InitContainers = append(pod.Spec.InitContainers, corev1.Container{
Name: nodejsInitContainerName,
Image: nodeJSSpec.Image,
Command: []string{"cp", "-a", "/autoinstrumentation/.", nodejsInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", nodejsInstrMountPath},
Resources: nodeJSSpec.Resources,
VolumeMounts: []corev1.VolumeMount{{
Name: nodejsVolumeName,
Expand Down
4 changes: 2 additions & 2 deletions pkg/instrumentation/nodejs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestInjectNodeJSSDK(t *testing.T) {
{
Name: "opentelemetry-auto-instrumentation-nodejs",
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-nodejs"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-nodejs"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-nodejs",
MountPath: "/otel-auto-instrumentation-nodejs",
Expand Down Expand Up @@ -118,7 +118,7 @@ func TestInjectNodeJSSDK(t *testing.T) {
{
Name: "opentelemetry-auto-instrumentation-nodejs",
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-nodejs"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-nodejs"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-nodejs",
MountPath: "/otel-auto-instrumentation-nodejs",
Expand Down
28 changes: 14 additions & 14 deletions pkg/instrumentation/podmutator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ func TestMutatePod(t *testing.T) {
{
Name: nodejsInitContainerName,
Image: "otel/nodejs:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", nodejsInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", nodejsInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: nodejsVolumeName,
MountPath: nodejsInstrMountPath,
Expand Down Expand Up @@ -849,7 +849,7 @@ func TestMutatePod(t *testing.T) {
{
Name: nodejsInitContainerName,
Image: "otel/nodejs:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", nodejsInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", nodejsInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: nodejsVolumeName,
MountPath: nodejsInstrMountPath,
Expand Down Expand Up @@ -1176,7 +1176,7 @@ func TestMutatePod(t *testing.T) {
{
Name: pythonInitContainerName,
Image: "otel/python:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", pythonInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", pythonInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: pythonVolumeName,
MountPath: pythonInstrMountPath,
Expand Down Expand Up @@ -1364,7 +1364,7 @@ func TestMutatePod(t *testing.T) {
{
Name: pythonInitContainerName,
Image: "otel/python:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", pythonInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", pythonInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: pythonVolumeName,
MountPath: pythonInstrMountPath,
Expand Down Expand Up @@ -1713,7 +1713,7 @@ func TestMutatePod(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "otel/dotnet:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", dotnetInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", dotnetInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: dotnetInstrMountPath,
Expand Down Expand Up @@ -1893,7 +1893,7 @@ func TestMutatePod(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "otel/dotnet:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", dotnetInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", dotnetInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: dotnetInstrMountPath,
Expand Down Expand Up @@ -2082,7 +2082,7 @@ func TestMutatePod(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "otel/dotnet:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", dotnetInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", dotnetInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: dotnetInstrMountPath,
Expand Down Expand Up @@ -3357,7 +3357,7 @@ func TestMutatePod(t *testing.T) {
{
Name: nodejsInitContainerName,
Image: "otel/nodejs:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", nodejsInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", nodejsInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: nodejsVolumeName,
MountPath: nodejsInstrMountPath,
Expand All @@ -3366,7 +3366,7 @@ func TestMutatePod(t *testing.T) {
{
Name: pythonInitContainerName,
Image: "otel/python:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", pythonInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", pythonInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: pythonVolumeName,
MountPath: pythonInstrMountPath,
Expand All @@ -3375,7 +3375,7 @@ func TestMutatePod(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "otel/dotnet:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", dotnetInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", dotnetInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: dotnetInstrMountPath,
Expand Down Expand Up @@ -4012,7 +4012,7 @@ func TestMutatePod(t *testing.T) {
{
Name: nodejsInitContainerName,
Image: "otel/nodejs:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", nodejsInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", nodejsInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: nodejsVolumeName,
MountPath: nodejsInstrMountPath,
Expand All @@ -4021,7 +4021,7 @@ func TestMutatePod(t *testing.T) {
{
Name: pythonInitContainerName,
Image: "otel/python:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", pythonInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", pythonInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: pythonVolumeName,
MountPath: pythonInstrMountPath,
Expand All @@ -4030,7 +4030,7 @@ func TestMutatePod(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "otel/dotnet:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", dotnetInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", dotnetInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: dotnetInstrMountPath,
Expand Down Expand Up @@ -4898,7 +4898,7 @@ func TestMutatePod(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "otel/dotnet:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", dotnetInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", dotnetInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: dotnetInstrMountPath,
Expand Down
2 changes: 1 addition & 1 deletion pkg/instrumentation/python.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func injectPythonSDK(pythonSpec v1alpha1.Python, pod corev1.Pod, index int) (cor
pod.Spec.InitContainers = append(pod.Spec.InitContainers, corev1.Container{
Name: pythonInitContainerName,
Image: pythonSpec.Image,
Command: []string{"cp", "-a", "/autoinstrumentation/.", pythonInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", pythonInstrMountPath},
Resources: pythonSpec.Resources,
VolumeMounts: []corev1.VolumeMount{{
Name: pythonVolumeName,
Expand Down
8 changes: 4 additions & 4 deletions pkg/instrumentation/python_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func TestInjectPythonSDK(t *testing.T) {
{
Name: "opentelemetry-auto-instrumentation-python",
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-python"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-python"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-python",
MountPath: "/otel-auto-instrumentation-python",
Expand Down Expand Up @@ -134,7 +134,7 @@ func TestInjectPythonSDK(t *testing.T) {
{
Name: "opentelemetry-auto-instrumentation-python",
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-python"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-python"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-python",
MountPath: "/otel-auto-instrumentation-python",
Expand Down Expand Up @@ -211,7 +211,7 @@ func TestInjectPythonSDK(t *testing.T) {
{
Name: "opentelemetry-auto-instrumentation-python",
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-python"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-python"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-python",
MountPath: "/otel-auto-instrumentation-python",
Expand Down Expand Up @@ -287,7 +287,7 @@ func TestInjectPythonSDK(t *testing.T) {
{
Name: "opentelemetry-auto-instrumentation-python",
Image: "foo/bar:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", "/otel-auto-instrumentation-python"},
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-python"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-python",
MountPath: "/otel-auto-instrumentation-python",
Expand Down
6 changes: 3 additions & 3 deletions pkg/instrumentation/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ func TestInjectNodeJS(t *testing.T) {
{
Name: nodejsInitContainerName,
Image: "img:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", nodejsInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", nodejsInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: nodejsVolumeName,
MountPath: nodejsInstrMountPath,
Expand Down Expand Up @@ -746,7 +746,7 @@ func TestInjectPython(t *testing.T) {
{
Name: pythonInitContainerName,
Image: "img:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", pythonInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", pythonInstrMountPath},
VolumeMounts: []corev1.VolumeMount{{
Name: pythonVolumeName,
MountPath: pythonInstrMountPath,
Expand Down Expand Up @@ -865,7 +865,7 @@ func TestInjectDotNet(t *testing.T) {
{
Name: dotnetInitContainerName,
Image: "img:1",
Command: []string{"cp", "-a", "/autoinstrumentation/.", dotnetInstrMountPath},
Command: []string{"cp", "-r", "/autoinstrumentation/.", dotnetInstrMountPath},
Copy link
Contributor

@jaronoff97 jaronoff97 Mar 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is cp -r available on all of the distributions we use? I tested it on my mac and it failed... i guess if our e2e tests are passing thats reason enough for me to believe it does work, but have you tested this locally on a cluster?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we use busybox everywhere, so it should work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah i think just some extra confirmation that this worked on a local kind cluster would be good

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current E2E are testing this if I'm not wrong @IshwarKanse

Copy link
Contributor

@IshwarKanse IshwarKanse Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the current e2e tests are not checking the status of the containers in the pod. I have fixed that with the PR #2702 (pending merge )with the changes, we check all the containers in the instrumented app pod are running and the test will fail if any of the container fails.

VolumeMounts: []corev1.VolumeMount{{
Name: dotnetVolumeName,
MountPath: dotnetInstrMountPath,
Expand Down
Loading