Skip to content

Add 'agent.dependencyURLs' array and download listed URLs via initContainer to the agent's deployment folder - #290

Open
htreu wants to merge 1 commit into
instana:mainfrom
htreu:agent-external-dependency-urls
Open

Add 'agent.dependencyURLs' array and download listed URLs via initContainer to the agent's deployment folder#290
htreu wants to merge 1 commit into
instana:mainfrom
htreu:agent-external-dependency-urls

Conversation

@htreu

@htreu htreu commented Aug 6, 2025

Copy link
Copy Markdown
Contributor

hint: AI generated code

Why

Instana agent might require external Java dependencies, which can not be served by IBM directly nor shipped with the agent installation. Those additional dependencies were previously downloaded via manual initContainer into the agent's container. The manual initContainer was part of the static agent YAML deployment. Since this is deprecated and about to be removed end of 2025, this PR introduces the replacement option for external Java dependencies.

What

Provide a new array option "dependencyURLs" to list external jar files, which get downloaded into the Agent's /opt/instana/agent/deploy folder.

References

Checklist

  • Backwards compatible?
  • Release notes in public docs updated?
  • unit/e2e test coverage added or updated?

Note: Remember to run a helm chart release after the the operator release to make the changes available thru helm.

…tainer to the agent's deployment folder

    hint: AI generated code
@htreu
htreu requested a review from a team as a code owner August 6, 2025 13:53

@konrad-ohms konrad-ohms left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the feature update.
Could you open the PR on the main project instead of your fork? Otherwise CI is not running by default.

Also, there are a few changes required in existing files (see above) and an addition is necessary in an existing test:

$ git diff pkg/k8s/object/builders/agent/daemonset/daemonset_test.go
diff --git i/pkg/k8s/object/builders/agent/daemonset/daemonset_test.go w/pkg/k8s/object/builders/agent/daemonset/daemonset_test.go
index 5c3e2a8..1f36e05 100644
--- i/pkg/k8s/object/builders/agent/daemonset/daemonset_test.go
+++ w/pkg/k8s/object/builders/agent/daemonset/daemonset_test.go
@@ -344,6 +344,7 @@ func TestDaemonSetBuilder_getVolumes(t *testing.T) {
                gomock.Eq(volume.TlsVolume),
                gomock.Eq(volume.RepoVolume),
                gomock.Eq(volume.NamespacesDetailsVolume),
+               gomock.Eq(volume.DeployVolume),
        ).Return(expectedVolumes, expectedVolumeMounts)

        db := &daemonSetBuilder{

A context question.
Will it be sufficient to download artifacts in the /instana folder?
Your example will pull files like this

curl https://repo1.maven.org/maven2/mysql/mysql-connector-java/8.0.28/mysql-connector-java-8.0.28.jar --output /instana/mysql-connector-java-8.0.28.jar && curl https://repo1.maven.org/maven2/org/postgresql/postgresql/42.3.3/postgresql-42.3.3.jar --output /instana/postgresql-42.3.3.jar```

In the final agent pod, the files are available in `/opt/instana/agent/deploy`.
I am not fully sure, if sensors would be fine with that location or if that must be configurable/somewhere else.

Also, I guess this would be required for the remote agent deployment as well? See pkg/k8s/object/remote-agent.

But other than the remarks, the feature seem to work in general on my dev OCP cluster, thanks :+1: 

return []corev1.Container{
{
Name: "init-dependency-downloader",
Image: "registry.access.redhat.com/ubi8/ubi:latest",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Image: "registry.access.redhat.com/ubi8/ubi:latest",
Image: "registry.access.redhat.com/ubi9/ubi-minimal:latest",

We should use the same base image as on the operator level, the minimal image is sufficient as well and smaller in size.

}

// Create builder
builder := NewDaemonSetBuilder(agent, false, status.NewAgentStatusManager())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
builder := NewDaemonSetBuilder(agent, false, status.NewAgentStatusManager())
builder := NewDaemonSetBuilder(agent, false, status.NewAgentStatusManager(nil, nil))

The constructor requires parameters, but you can pass in nil values. Try running make, it will fail without these parameters.

Comment on lines +8 to +18
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

instanav1 "github.com/instana/instana-agent-operator/api/v1"
"github.com/instana/instana-agent-operator/pkg/k8s/operator/status"
"github.com/instana/instana-agent-operator/pkg/pointer"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
instanav1 "github.com/instana/instana-agent-operator/api/v1"
"github.com/instana/instana-agent-operator/pkg/k8s/operator/status"
"github.com/instana/instana-agent-operator/pkg/pointer"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
appsv1 "k8s.io/api/apps/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
instanav1 "github.com/instana/instana-agent-operator/api/v1"
"github.com/instana/instana-agent-operator/pkg/k8s/operator/status"

There are unused imports for corev1 and pointer, please remove those.

Comment thread docs/dependency-urls.md
- The original filenames from the URLs will be preserved
- You can specify multiple dependency URLs, and all files will be downloaded to the same directory
- Make sure the URLs are accessible from the Kubernetes cluster where the agent is running
- For security reasons, consider using HTTPS URLs and trusted sources No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, we should add that to the helm chart's readme as well once we have that merged.

http:
enabled: true

# Made with Bob

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

probably we won't want IDE comments on public examples, in source code I don't think it's an issue, but I guess it adds unneeded clutter to example yamls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants