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

Adding readiness probe for the payload-tracker api pods [RHCLOUD-19463] #134

Closed
wants to merge 1 commit into from

Conversation

tahmidefaz
Copy link
Member

What?

Adds a readiness probe for the payload tracker API pods.

Jira

Why?

This is an app-sre requirement.

How?

Added the readiness probe config in the deploy.yml

Testing

N/A

Anything Else?

Any other notes about the PR that would be useful for the reviewer.

Secure Coding Practices Checklist Link

Secure Coding Checklist

  • Input Validation
  • Output Encoding
  • Authentication and Password Management
  • Session Management
  • Access Control
  • Cryptographic Practices
  • Error Handling and Logging
  • Data Protection
  • Communication Security
  • System Configuration
  • Database Security
  • File Management
  • Memory Management
  • General Coding Practices

Copy link
Contributor

@CodyWMitchell CodyWMitchell left a comment

Choose a reason for hiding this comment

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

I believe the health endpoint is on /health, not /v1/health (double check me on that)

Also, does it make sense to have a separate endpoint from the liveness check?

Otherwise, LGTM

@tahmidefaz
Copy link
Member Author

tahmidefaz commented Nov 11, 2022

I believe the health endpoint is on /health, not /v1/health (double check me on that)

Also, does it make sense to have a separate endpoint from the liveness check?

Otherwise, LGTM

Hey @CodyWMitchell , good question! I was actually following the livenessProbe config in that same file. The readinessProbe section is that I am adding here is identical with the livenessProbe section in the same file. Also, it looks like there is already a livenessProbe and readinessProbe section in the clowdapp.yml that is using just the /heath endpoint, not sure what's going on there tbh 🤔
Also, one single endpoint for the liveness and readiness probe should be fine for now. We can add a separate one later if needed.

Copy link
Contributor

@aleccohan aleccohan left a comment

Choose a reason for hiding this comment

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

@tahmidefaz Based on the deploy.yml in app-interface for payload-tracker-go it looks like the clowder config file being used for deployment is /deployments/clowdapp.yml rather than /deployments/deploy.yml so these changes should be made there instead.

@tahmidefaz
Copy link
Member Author

@tahmidefaz Based on the deploy.yml in app-interface for payload-tracker-go it looks like the clowder config file being used for deployment is /deployments/clowdapp.yml rather than /deployments/deploy.yml so these changes should be made there instead.

Got it! Looks like there is already a readiness probe for the api in clowdapp.yml. Closing this PR and marking the ticket as done. Thank you!

@tahmidefaz tahmidefaz closed this Nov 15, 2022
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.

None yet

3 participants