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

Allow to define custom scripts in independent Elastic Agents #1822

Merged
merged 28 commits into from
May 13, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented May 7, 2024

Part of #787

This PR allows to define custom scripts for independent Elastic Agents. This custom scripts can be used to install required software to run the required tests successfully. For instance some libraries as it happens with oracle package.

Example of the new keys added into the configuration file:

agent:
  custom_script: |
    mkdir -p /my/path
    wget ...
  pre_start_script: |
    export PATH=${PATH}:/my/path

Now the files required to spin a new independent Elastic Agent using containers has been migrated to go-resources. Files generated can be checked within the profile folder:

 $ ls -l ~/.elastic-package/profiles/default/deployer/docker-agent-oracle-memory-61511/
total 12
-rw-r--r-- 1 user user 1046 may  8 16:15 docker-agent-base.yml
-rw-r--r-- 1 user user  203 may  8 16:15 Dockerfile
-rwxr-xr-x 1 user user  764 may  8 16:15 script.sh

If custom scripts are required, elastic-package builds a new Docker image adding the suffix -custom to the tag. Example:

 $ docker images |grep custom
docker.elastic.co/elastic-agent/elastic-agent-complete   8.13.2-custom     ee55b4ece29b   22 hours ago    3.95GB

Testing

Added a new test package test/packages/parallel/oracle based on the one from test/packages/with-custom-agent/oracle but with some changes:

  • Removed custom "agent" deploy (deleted folder).
  • Added just the oracle service in _dev/deploy/docker.
  • Added the required agent settings in the configuration file for system tests

This package does not work when tested with the Elastic stack running from the stack since it is missing some required libraries/dependencies the container.

  • Process to test this locally:
elastic-package stack up -v -d
export ELASTIC_PACKAGE_TEST_ENABLE_INDEPENDENT_AGENT=true

# test new package with independent Elastic Agents
pushd test/packages/parallel/oracle
elastic-package test system -v
popd

# test old package - it should keep using previous Custom Agents with servicedeployer
pushd test/packages/with-custom-agent/oracle
elastic-package test system -v
popd

elastic-package stack down -v

@mrodm mrodm self-assigned this May 7, 2024
Comment on lines +97 to +100
if [[ "$independent_agent" == "false" && "$package_name" == "oracle" ]]; then
echoerr "Package \"${package_name}\" skipped: not supported with Elastic Agent running in the stack (missing required software)."
continue
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Required exception, since CI tests this package with both:

  • Generic Elastic stack started with elastic-package stack up
  • Independent Elastic Agents (setting env. variable)

Elastic Agent from the stack does not have the required libraries for this package.

@@ -13,6 +13,27 @@ const (
elasticAgentTagsEnv = "ELASTIC_AGENT_TAGS"
)

type AgentSettings struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to an independent struct in order to be used as embbeded struct here and in the test config. That makes possible to copy all fields at one in https://github.com/elastic/elastic-package/pull/1822/files#diff-de89bc59d9fa58f6220a67bda14afddc8f04ef1b816e81c2ca851711625f5834R339

Comment on lines 13 to 16
pre_start_script: |
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/opt/oracle/instantclient_21_4"
export PATH="${PATH}:/opt/oracle/instantclient_21_4"
cd /opt/oracle/instantclient_21_4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested locally without this pre-start script and it also worked. However, I'd prefer to keep it since it was set in the Dockerfile too here:

RUN export LD_LIBRARY_PATH=/opt/oracle/instantclient_21_4:$LD_LIBRARY_PATH && export PATH=/opt/oracle/instantclient_21_7:$PATH
ENV LD_LIBRARY_PATH "${LD_LIBRARY_PATH}:/opt/oracle/instantclient_21_4"
ENV PATH "${PATH}:/opt/oracle/instantclient_21_4"
WORKDIR /opt/oracle/instantclient_21_4

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can use a simpler package for testing. It could even maybe enough to have a script that writes a file, and a config that reads this file with a log input.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new test package under test/packages/other with a minimal configuration to use these scripts.

About the test package oracle , do you think it should be kept under parallel as an example for the migration ?

@mrodm mrodm marked this pull request as ready for review May 8, 2024 14:41
@mrodm mrodm requested a review from a team May 8, 2024 14:41
vars:
hosts:
- "oracle://sys:Oradoc_db1@elastic-package-service-oracle-1:1521/ORCLCDB.localdomain?sysdba=1"
agent:
Copy link
Contributor Author

@mrodm mrodm May 8, 2024

Choose a reason for hiding this comment

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

According to the tests in this PR, to update the oracle package in the integrations repository it should be needed:

  • add this key agent to each of the test system configuration files
  • update the hosts value to set as host elastic-package-service-oracle-1.
  • remove _dev/deploy/agent folders
  • replace agent folders with _dev/deploy/docker folder (something similar to this PR) with just the service definition (oracle).

@@ -0,0 +1,16 @@
vars:
hosts:
- "oracle://sys:Oradoc_db1@elastic-package-service-oracle-1:1521/ORCLCDB.localdomain?sysdba=1"
Copy link
Contributor Author

@mrodm mrodm May 8, 2024

Choose a reason for hiding this comment

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

This is required to be changed since the container name changes from oracle to elastic-package-service-oracle-1.

-    - "oracle://sys:Oradoc_db1@oracle:1521/ORCLCDB.localdomain?sysdba=1"
+    - "oracle://sys:Oradoc_db1@elastic-package-service-oracle-1:1521/ORCLCDB.localdomain?sysdba=1"

services:
elastic-agent:
hostname: ${AGENT_HOSTNAME}
{{ if ne $custom_script "" }}
image: "${ELASTIC_AGENT_IMAGE_REF}-custom"
Copy link
Member

Choose a reason for hiding this comment

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

Should we use some other suffix to avoid conflicts? The image tag will be used if it exists, and it could be an unexpected one. Maybe we can use a checksum of the scripts.

Suggested change
image: "${ELASTIC_AGENT_IMAGE_REF}-custom"
image: "${ELASTIC_AGENT_IMAGE_REF}-{{ $some_id }}"

Copy link
Contributor Author

@mrodm mrodm May 10, 2024

Choose a reason for hiding this comment

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

Added as a suffix the MD5 sum of the Dockerfile created.
image: "elastic-package-test-elastic-agent-complete:{{ $stack_version }}-{{ $dockerfile_hash }}"

The result is images like this:

elastic-package-test-elastic-agent-complete:8.13.2-861d45f9a2042f22c52105b4b3d1708a

- "oracle://sys:Oradoc_db1@elastic-package-service-oracle-1:1521/ORCLCDB.localdomain?sysdba=1"
agent:
runtime: docker
custom_script: |
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this an object?

Suggested change
custom_script: |
custom_script:
content: |

This could be useful to support referencing scripts in the future, or even other interpreters:

  custom_script:
    language: bash
    source: ./custom.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added for custom_script (now it's named provisioning_script) and also for pre_start_script.

For pre_start_script , it is just allowed language sh, since it is injected directly those commands into the start script (entrypoint.sh).

internal/agentdeployer/_static/dockerfile.tmpl Outdated Show resolved Hide resolved
internal/agentdeployer/_static/dockerfile.tmpl Outdated Show resolved Hide resolved
Comment on lines 13 to 16
pre_start_script: |
export LD_LIBRARY_PATH="${LD_LIBRARY_PATH}:/opt/oracle/instantclient_21_4"
export PATH="${PATH}:/opt/oracle/instantclient_21_4"
cd /opt/oracle/instantclient_21_4
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can use a simpler package for testing. It could even maybe enough to have a script that writes a file, and a config that reads this file with a log input.

Comment on lines 289 to 290
"kibana_host": "https://kibana:5601",
"fleet_url": "https://fleet-server:8220",
Copy link
Member

Choose a reason for hiding this comment

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

Umm, would that work with the serverless provider?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I took as a basis the docker-custom base template, and according to that I'd say custom agents would not work with the serverless provider.

environment:
- FLEET_ENROLL=1
- FLEET_URL=https://fleet-server:8220
- KIBANA_HOST=https://kibana:5601
- FLEET_TOKEN_POLICY_NAME=${FLEET_TOKEN_POLICY_NAME}

I'll try to give it a chance to make it work.

Copy link
Member

Choose a reason for hiding this comment

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

Well, we can leave this for a different change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it would be better. It would require a few more changes. Afterwards, I'll create a PR to add Serverless support for independent Elastic Agents.

@mrodm
Copy link
Contributor Author

mrodm commented May 10, 2024

/test

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@@ -0,0 +1,40 @@
format_version: 1.0.0
name: oracle
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it's interesting to keep this for reference until oracle package is updated in integrations. WDYT ?

Comment on lines +4 to +14
- json:
field: message
target_field: target
- set:
field: error.message
value: not present target
if: ctx?.target == null
on_failure:
- set:
field: error.message
value: '{{ _ingest.on_failure_message }}'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If message is not a JSON (written by the entrypoint script) , system tests are going to fail.

@mrodm mrodm requested a review from jsoriano May 13, 2024 09:33
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

👍

@mrodm mrodm merged commit 5c414a6 into elastic:main May 13, 2024
3 checks passed
@mrodm mrodm deleted the add-custom-script-agents branch May 13, 2024 17:45
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.

3 participants