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

Provide environment variable to disable wait-for-it condition in deployments #4377

Merged
merged 2 commits into from
Nov 3, 2023

Conversation

dhaley
Copy link
Collaborator

@dhaley dhaley commented Nov 2, 2023

This is to fix bug in stratus where celery container halted as wait-for-it script couldn't make socket connection to main connection.

What's this PR do?

This fix disables wait-for-it scripts that are not needed as managed services such as Postgres, Redis are not in docker and are already running. So wait-for-it scrips are not needed in Stratus environment.

How should this be manually tested?

Build Docker image of seed and deploy to Stratus.

What are the relevant tickets?

Screenshots (if appropriate)

@dhaley dhaley added the Bug label Nov 2, 2023
@dhaley dhaley added this to the SEED Maintenance & Support milestone Nov 2, 2023
@dhaley dhaley requested a review from kflemin November 2, 2023 15:14
@dhaley dhaley self-assigned this Nov 2, 2023
@kflemin kflemin requested a review from nllong November 2, 2023 16:37
Copy link
Member

@nllong nllong left a comment

Choose a reason for hiding this comment

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

cool cool, one change requested...

Still wondering why this wouldn't work. It seems like we are changing our code to handle a specific setup instead of figuring out why the setup doesn't work with our code...

then
POSTGRES_ACTUAL_HOST=$POSTGRES_HOST
# Check if 'STRATUS_MANAGED_SERVICES_ENABLED' is not set or if its value is 'TRUE'
if [[ "${STRATUS_MANAGED_SERVICES_ENABLED}" == "on" ]]; then
Copy link
Member

Choose a reason for hiding this comment

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

can we name this something not NREL-specific... others use this code as well. Something like DISABLE_SERVICE_CHECKS_ON_START or something? And add a warning that one should not do this unless they are sure their services are running.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. That's a better name. Let me fix that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed.

Copy link
Member

Choose a reason for hiding this comment

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

thanks, this is much more applicable to our external users too!

@nllong nllong changed the title disable wait-for-it in Stratus - managed services are always running. Provide environment variable to disable wait-for-it condition in deployments Nov 2, 2023
@nllong nllong added Feature Add this label to new features. This will be reflected in the change log when generated. and removed Bug labels Nov 2, 2023
@dhaley dhaley force-pushed the feature/stratus-faster-startup-time branch from 8ca98e5 to a1815d1 Compare November 2, 2023 19:32
@nllong nllong merged commit ebd4aa8 into develop Nov 3, 2023
8 checks passed
@nllong nllong deleted the feature/stratus-faster-startup-time branch November 3, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Add this label to new features. This will be reflected in the change log when generated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants