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

Write container-paths.yml into the STATE_PATH #4995

Merged
merged 10 commits into from
Jul 2, 2024

Conversation

blakerouse
Copy link
Contributor

@blakerouse blakerouse commented Jun 24, 2024

What does this PR do?

Adjusts the container subcommand to write the container-paths.yml into the STATE_PATH on startup.

Why is it important?

When running the Elastic Agent container with --read-only it is not possible for the container to write the container-paths.yml files into the root filesystem of the container.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in ./changelog/fragments using the changelog tool
  • [ ] I have added an integration test or an E2E test

How to test this PR locally

$ PLATFORMS="linux/arm64" PACKAGES="docker" SNAPSHOT=true EXTERNAL=true mage package
...
$ docker run --rm --mount type=tmpfs,destination=/state -e STATE_PATH=/state --read-only docker.elastic.co/beats/elastic-agent:8.15.0-SNAPSHOT

In another shell with the container running verify that the status command works.

$ docker exec -it ${container_id} bash
elastic-agent@7d7447b8f608:~$ elastic-agent status
┌─ fleet
│  └─ status: (STOPPED) Not enrolled into Fleet
└─ elastic-agent
   └─ status: (HEALTHY) Running

@blakerouse blakerouse added Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team backport-skip labels Jun 24, 2024
@blakerouse blakerouse self-assigned this Jun 24, 2024
@blakerouse blakerouse requested a review from a team as a code owner June 24, 2024 21:52
@elasticmachine
Copy link
Contributor

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@pierrehilbert pierrehilbert requested review from michalpristas and pchila and removed request for faec and leehinman June 25, 2024 07:18
@ycombinator
Copy link
Contributor

ycombinator commented Jun 26, 2024

@blakerouse Looking at the How to test this PR locally section of this PR, I see that we'd need to run Agent in a read-only container like so:

docker run --rm --mount type=tmpfs,destination=/state -e STATE_PATH=/state --read-only docker.elastic.co/beats/elastic-agent:8.15.0-SNAPSHOT

Just want to confirm that, even with the changes in this PR, the -mount type=tmpfs,destination=/state -e STATE_PATH=/state arguments will also be needed? I don't think there's any getting around them but just want to confirm that so we can then document this requirement along with the caveat you mentioned in this PR about not being able to use elastic-agent sub-commands inside the read-only container.

cc: @kilfoyle

@blakerouse blakerouse marked this pull request as draft June 27, 2024 15:19
@blakerouse
Copy link
Contributor Author

Moved back to draft, as I adjust this to write the container-paths.yml into the STATE_PATH.

@blakerouse blakerouse marked this pull request as ready for review June 27, 2024 18:42
@blakerouse blakerouse changed the title Don't fail on not writing container-paths.yml Write container-paths.yml into the STATE_PATH Jun 27, 2024
@blakerouse
Copy link
Contributor Author

I have updated this PR to write the container-paths.yml into the STATE_PATH. This provides a much better experience then the implementation this PR previously had, as now all sub-commands once exec into the container still work.

@blakerouse blakerouse enabled auto-merge (squash) July 1, 2024 14:54
@blakerouse blakerouse added backport-v8.14.0 Automated backport with mergify and removed backport-skip labels Jul 1, 2024
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarQube

@blakerouse blakerouse disabled auto-merge July 2, 2024 14:49
@ycombinator
Copy link
Contributor

SonarQube, as usual, isn't able to account for coverage from integration tests so that check is falsely failing. Other checks are passing, so force merging.

@ycombinator ycombinator merged commit c46a379 into elastic:main Jul 2, 2024
12 of 13 checks passed
mergify bot pushed a commit that referenced this pull request Jul 2, 2024
* Don't write container paths.

* Clean up.

* Add changelog.

* Store container-paths.yml in STATE_PATH.

* Update 1719266012-Don't-fail-when-unable-to-write-container-paths.yaml

* fix issue with empty value

* Fix test.

(cherry picked from commit c46a379)

# Conflicts:
#	testing/integration/container_cmd_test.go
blakerouse pushed a commit that referenced this pull request Jul 2, 2024
@blakerouse blakerouse deleted the container-read-only branch July 3, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.14.0 Automated backport with mergify Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants