-
Notifications
You must be signed in to change notification settings - Fork 210
Fix dir name for moving elasticsearch connectors #11106
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
Conversation
|
This pull request does not have a backport label. Could you fix it @artem-shelkovnikov? 🙏
|
|
Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane) |
|
I can still see a reference to the old naming here:
Should we also change it? |
|
Thanks @pierrehilbert! I'm actually not sure how this part works though - we do the unpacking of a zip in I've done the change, but I'm not aware what it did. |
|
I believe the build's failing because we have
The GH action won't trigger an update until a new unified release snapshot is produced on We could force this change with broken CI through (and leaving @pierrehilbert @blakerouse @michalpristas @swiatekm @pchila - anyone have a better way to resolve this situation? |
|
Alternatively for connectors we can revert the change, fix everything and re-revert it back once we're sure that the change is compatible with agentless, but it won't be too trivial either IMO |
|
@ebeahan I believe your path forward is the best path. I don't like it, but seems like the simplest path without wasting time. |
blakerouse
left a comment
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.
This change looks correct. Won't really know until merged and a new DRA is produced, which then can bump the .package-version.
I am good with this change, to get this in.
@ebeahan We can disable the pinning and package "bleeding edge" versions of the dependencies in CI builds by setting USE_PACKAGE_VERSION to |
@artem-shelkovnikov |
|
@pchilla thanks for the explanation! What's our plan of action now? Is there anything I can do? |
@artem-shelkovnikov |
|
Now seeing two different issues in CI:
Error: FetchProjectBinary failed for agentbeat on windows/amd64: checksum of file agentbeat-9.3.0-SNAPSHOT-windows-x86_64.zip does not match expected checksum of 12d8e8273bd74d05aab4682313225b9493bbb039a2122cf3eaee30f090458b2221eba62a84080cecad934f752d96197639b5cf3407617df97384f6d23bf18cecedit: add error |
|
Created a PR that should remove the race conditions while downloading packages with USE_PACKAGE_VERSION=false |
|
We've addressed the bug on the Agent side. Now seeing new error in Packaging: 5.073 make: *** /usr/share/connectors: Not a directory. Stop.
--
| 5.073 make: Entering directory '/'
| 5.073 make: Leaving directory '/'
| ------
| Dockerfile:132
| --------------------
| 131 \| COPY --from=home /usr/share/elastic-agent/NOTICE.txt /licenses
| 132 \| >>> RUN apk add --no-cache git make python-3.11 py3.11-pip && \
| 133 \| >>> unzip /usr/share/elastic-agent/data/service/connectors-*.zip -d /usr/share/elastic-agent/data/service && \
| 134 \| >>> mv /usr/share/elastic-agent/data/service/connectors-* /usr/share/connectors && \
| 135 \| >>> PYTHON=python3.11 make -C /usr/share/connectors clean install install-agent && \
| 136 \| >>> chmod 0755 /usr/share/elastic-agent/data/elastic-agent-*/components/connectors
| 137 \|
| --------------------I compared inflating the the zips for @artem-shelkovnikov It's not just renaming the directory, but the entire directory structure has changed. Is that expected? Before we were able to move the entire dir using |
💛 Build succeeded, but was flaky
Failed CI Steps
History
|
|
Packaging succeeded in CI. We have one remaining failure due to a known flakey test: #10917. @artem-shelkovnikov any remaining work? I can merge with the unrelated flakey failure. |
|
@ebeahan thanks for giving it a look! I think it's okay, but one question - before there was a failure due to connectors-py unable to start in the tests. I've fixed everything seemingly, but worried that I've missed anything and on deployment to Agentless things can break. Do you think the PR is safe to merge given that the tests for the image passed, or should we do manual testing in some manner? |
The only test that checks So as far as supporting inputs |
|
@pchila thanks! If I want to do integration tests with the image, what's the best way forward? Pulling the artefact from the step locally and running Elasticsearch+Kibana+this image locally? |
If you want to test the image manually, yes. I am not too sure what the exact setup looks like but if you already tested connectors included with elastic-agent in a k8s cluster the setup should be the same. |
|
Thanks, I will give it a manual test tomorrow! |
|
@artem-shelkovnikov we're still blocking unified release snapshots until we fix the agent packaging failures. Are we able to merge these changes to unblock? |
|
@ebeahan I've taken today to try to test this change with help from #agentless channel but were unable to fully test it. Here are my current thoughts:
I was not able to test this change in real Agentless or with real Agentless image. If that's okay to merge, let's do it. Otherwise we need help from somebody who knows how to test it end-to-end |
The Agent team discussed internally and feels confident in merging. I'll see if we can get a green CI run and merge. |
|
@ebeahan how can I learn when the change is available in staging agentless so that I could give it a manual test? |
* Fix dir name for moving elasticsearch connectors * Also change the pattern in packages.yml * Try USE_PACKAGE_VERSION=false * set USE_PACKAGE_VERSION to false * Update the unzip command to the correct folder * Update connectors.sh to point to the right dir --------- Co-authored-by: Eric Beahan <eric.beahan@elastic.co>
What does this PR do?
See: https://elastic.slack.com/archives/C0JFN9HJL/p1762485477102909 - there is a problem producing DRA artefacts for agentless images. The bug happens because the connectors were restructured and some folder names changed. In this case the name of the directory extracted from the DRA changed - and this PR updates it.
Why is it important?
DRA is broken, and we need to fix it.
Checklist
./changelog/fragmentsusing the changelog toolDisruptive User Impact
How to test this PR locally
Unfortunately, I'm not sure.
Related issues
Questions to ask yourself