Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Removing start, stop with ec2.py, adding validations #1191
base: main
Are you sure you want to change the base?
Removing start, stop with ec2.py, adding validations #1191
Changes from 15 commits
3b11c13
6034c5e
d1f1756
f42f872
2542232
edf85f3
3e95e4c
cb70032
5fe844c
937e7a2
2b23ff0
44aa71f
711d50b
4c694e7
62cc490
5de70be
77f1f4a
a4241fc
0bff456
e669887
85fc3e7
d7b24c7
4499dcf
3dd967d
45b2908
d890e5d
8eeaf9a
eb8955c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
is this right translation from the old bash script:
VSOCK_LOG_LEVEL=${VSOCK_LOG_LEVEL:-3}
?
(granted don't quite understand this bash script)
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.
if VSOCK_LOG_LEVEL ? then VSOCK_LOG_LEVEL else 3
there is no way to set VSOCK_LOG_LEVEL though.
So I added if debug_mode ? then set to 3 otherwise 1.
In future ticket, we will be adding more tracing on debug mode.
Plan is to evaluate adding
echo 1 > /sys/kernel/debug/tracing/events/vsock/enable
echo 1 > /sys/kernel/debug/tracing/tracing_on
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.
why do we need to sleep here? doesn't
subprocess.run
already run the command synchronously and will wait for its completion? For config server, it would be run as separate process so will never need to wait for it anyway?or is this to wait for config server to startup so you could do validations? if so, you might wanna consider making it more robust (are we 100% sure 5 seconds wait is enough?), or at the very least, logs something during this 5 second wait to inform the customer this script is still running but waiting for something.
or maybe it should be in a loop and every 5 seconds the validation script will try to connect to the server until it's successful.
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.
or is this to wait for config server to startup so you could do validations? i
Yes. we can "Change to while loop if required. " as mentioned in 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.
would be helpful for debugging to have the ability to pass this in.
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.
why?
We need to build an EIF and register the EIF in core. Why not just use the ami build along with EIF? otherwise we have to copy the registered EIF and run that by SSH ing into the host right?
This file was deleted.
This file was deleted.