Skip to content

[CONTINT-4562] Container generator#1227

Merged
L3n41c merged 15 commits intomainfrom
lenaic/CONTINT-4562
Apr 11, 2025
Merged

[CONTINT-4562] Container generator#1227
L3n41c merged 15 commits intomainfrom
lenaic/CONTINT-4562

Conversation

@L3n41c
Copy link
Copy Markdown
Member

@L3n41c L3n41c commented Jan 31, 2025

What does this PR do?

A form of load can be a container that exists and exposes some data.

This generator aims to provide a way to start a generic container based on user's config.

Motivation

Test the agent performances when it monitors many containers

Related issues

Additional Notes

Follow-up of #1154.

@L3n41c L3n41c force-pushed the lenaic/CONTINT-4562 branch from 666c7dc to 657129d Compare January 31, 2025 09:30
Copy link
Copy Markdown
Collaborator

@blt blt left a comment

Choose a reason for hiding this comment

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

I really like where this is headed.

@blt blt mentioned this pull request Feb 4, 2025
@L3n41c L3n41c force-pushed the lenaic/CONTINT-4562 branch from a1f4f67 to 0295411 Compare April 9, 2025 13:08
@L3n41c L3n41c marked this pull request as ready for review April 9, 2025 13:45
@L3n41c L3n41c requested a review from a team as a code owner April 9, 2025 13:45
Copy link
Copy Markdown
Collaborator

@blt blt left a comment

Choose a reason for hiding this comment

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

Some style comments on the commits. I would like to clarify what happens if the spawned container exits. As I read it the Container generator will not check for spawned container liveness and so if a spawned container fails the user won't know until stop_and_remove_container is called? There's potentially a long period of time between startup and shutdown so an earlier warning -- of outright failure of lading -- is desirable. I could buy may-fail or must-not-fail approaches to this generator, but documenting which approach is taken here is helpful to the user.

Comment thread lading/src/generator.rs Outdated
Comment thread lading/src/generator/container.rs Outdated
Comment thread lading/src/generator/container.rs Outdated
Comment thread lading/src/generator/container.rs Outdated
Comment thread lading/src/generator/container.rs Outdated
Comment thread lading/src/generator/container.rs Outdated
Comment thread lading/src/generator/container.rs Outdated
Comment thread lading/src/generator/container.rs Outdated
Comment thread lading/src/generator/container.rs Outdated
@L3n41c
Copy link
Copy Markdown
Member Author

L3n41c commented Apr 10, 2025

@blt Thank you for your review!
I should have addressed all the comments.

I would like to clarify what happens if the spawned container exits. As I read it the Container generator will not check for spawned container liveness and so if a spawned container fails the user won't know until stop_and_remove_container is called? There's potentially a long period of time between startup and shutdown so an earlier warning -- of outright failure of lading -- is desirable.

I’ve also updated the PR so that lading is now periodically checking if the spawned containers are still running. If they’re not, a warning is issued.

Copy link
Copy Markdown
Collaborator

@blt blt left a comment

Choose a reason for hiding this comment

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

One small comment about using interval instead of repeatedly creating sleeps, otherwise looks good. I'll be happy to see this merge up after that's fixed.

Comment thread lading/src/generator/container.rs Outdated
@L3n41c L3n41c merged commit 6c6a683 into main Apr 11, 2025
21 checks passed
@L3n41c L3n41c deleted the lenaic/CONTINT-4562 branch April 11, 2025 13:54
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.

4 participants