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

add optional verification jobs after deploys #108

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

elrayle
Copy link
Collaborator

@elrayle elrayle commented Oct 29, 2024

Description

Add verification after deploy. The verification is optional for backward compatibility.

The verification-url is expected to be the app's health endpoint which should produce output in the format...

{
  "status": "OK",
  "version": "v2.0.0-dev-476113a964",
  "sha": "476113a9648fe208a388d867c93070d3c91e3654"
}

Follow on steps

  • release v3.2.0
  • update apps using the workflow to v3.2.0 and add verification URLs for each

@elrayle elrayle force-pushed the elr/verify-health branch 4 times, most recently from 6f40677 to 7a0d890 Compare October 31, 2024 02:43
@elrayle
Copy link
Collaborator Author

elrayle commented Oct 31, 2024

This is ready for review.

NOTE: A pause is required after deploy completes to allow the system to restart before trying to verify. Not sure it needs to be 5 minutes, but that resulted in a correct validation every time. Without the pause, validation would give a false negative.

@qtomlinson Is there ever a second URL to test the EU deploys? I'm wondering if only one validation is needed even if the deploy goes to US and EU.

Copy link
Contributor

@ljones140 ljones140 left a comment

Choose a reason for hiding this comment

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

Looks great.

I'll hold off approving as you need to change the version ref

runs-on: ubuntu-latest
steps:
- name: Pause for 5 minutes
run: sleep 300 # Sleep for 300 seconds (5 minutes)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will slow deploys, but I guess we have to live with it for now.

Will be interesting to see if we get timeouts once we go live. If not we may be able to reduce?

uses: actions/checkout@v4.1.1
with:
repository: 'clearlydefined/operations'
ref: 'elr/verify-health'
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to change this to the new version before we merging

@@ -44,10 +44,18 @@ on:
description: 'postfix to apply to the base name for the primary deploy site (e.g. -prod, -dev)'
required: true
type: string
primary-verification-url:
Copy link
Contributor

Choose a reason for hiding this comment

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

Great this is an input as it's going to be different for each app

run ./scripts/app-workflows/validate-deploy.sh "" "v2.999.0" "1234567890ABCDEF"
test_value 1 "$status"
test_value "Error: Invalid JSON string" "${lines[0]}"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Great to be thorough with the tests

uses: actions/checkout@v4.1.1
with:
repository: 'clearlydefined/operations'
ref: 'elr/verify-health'
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to change before merging

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.

2 participants