chore(testinfra): initial ami test #746
Conversation
soedirgo
force-pushed
the
bs/testinfra-ami
branch
2 times, most recently
from
aac3052 to
8619064
Compare
| from ec2instanceconnectcli.EC2InstanceConnectKey import EC2InstanceConnectKey | ||
| from time import sleep | ||
|
|
||
| postgresql_schema_sql_content = """ |
There was a problem hiding this comment.
We need to hardcode a lot of these since the setup logic lives in worker. This makes the tests brittle. One possible improvement would be to move some of the setup logic into AMI creation.
There was a problem hiding this comment.
The bulk of the content here is dependent on project specific data that'll only be available in the worker, no?
The improvements to be made here are in making the interface between the AMI and the worker more explicit, and testing on both sides (that the worker produces the right interface, and that the AMI here takes that interface and does the right things with it).
There was a problem hiding this comment.
To start with, we can add a note in the relevant spots on both sides that any change to one (worker config generation/postgres integration test) must be captured by an update and ideally a test in the other
There was a problem hiding this comment.
Yeah, what I mean is to move the file templating to supabase/postgres, so that UserData only contains things that need to be variable. I'll start with adding a note for now.
soedirgo
force-pushed
the
bs/testinfra-ami
branch
from
8619064 to
d6e79b5
Compare
| from ec2instanceconnectcli.EC2InstanceConnectKey import EC2InstanceConnectKey | ||
| from time import sleep | ||
|
|
||
| postgresql_schema_sql_content = """ |
There was a problem hiding this comment.
The bulk of the content here is dependent on project specific data that'll only be available in the worker, no?
The improvements to be made here are in making the interface between the AMI and the worker more explicit, and testing on both sides (that the worker produces the right interface, and that the AMI here takes that interface and does the right things with it).
| from ec2instanceconnectcli.EC2InstanceConnectKey import EC2InstanceConnectKey | ||
| from time import sleep | ||
|
|
||
| postgresql_schema_sql_content = """ |
There was a problem hiding this comment.
To start with, we can add a note in the relevant spots on both sides that any change to one (worker config generation/postgres integration test) must be captured by an update and ideally a test in the other
| ec2 = boto3.resource("ec2", region_name="ap-southeast-1") | ||
| images = list( | ||
| ec2.images.filter( | ||
| Filters=[{"Name": "name", "Values": ["supabase-postgres-ci-ami-test"]}] |
There was a problem hiding this comment.
We should be using the version string from the common.vars file here, yeah? Not sure which AMI this is currently set up to use?
There was a problem hiding this comment.
The test run will always publish an AMI with AMI name supabase-postgres-ci-ami-test. Subsequent runs will deregister older AMIs through the force_deregister option.
One alternative I haven't explored is to move the test run into a Packer provisioning step, that way we might not even need to publish the AMI. Can be done as a follow up.
|
I think it's better for the infra team to take over, since you have a better understanding of the requirements. |
| - name: Run tests | ||
| run: | | ||
| # TODO: use poetry for pkg mgmt | ||
| pip3 install boto3 boto3-stubs[essential] docker ec2instanceconnectcli pytest pytest-testinfra[paramiko,docker] requests | ||
| pytest -vv testinfra/test_ami.py | ||
|
|
||
| - name: Cleanup resources on build cancellation | ||
| if: ${{ cancelled() }} | ||
| run: | | ||
| aws ec2 --region ap-southeast-1 describe-instances --filters "Name=tag:packerExecutionId,Values=${GITHUB_RUN_ID}" --query "Reservations[].Instances[].InstanceId" --output text | xargs -I {} aws ec2 terminate-instances --instance-ids {} |
There was a problem hiding this comment.
since the setup to this point is the same as ami-release could we add these lines to that file instead & run it on PR to reduce duplication?
soedirgo
force-pushed
the
bs/testinfra-ami
branch
from
20449cb to
5505c2d
Compare
soedirgo
force-pushed
the
bs/testinfra-ami
branch
from
5505c2d to
ecf8fe0
Compare
|
Merging this for now - will dedupe some of the Action steps in another PR |
soedirgo
force-pushed
the
bs/testinfra-ami
branch
6 times, most recently
from
e802ea1 to
78a8d26
Compare
soedirgo
force-pushed
the
bs/testinfra-ami
branch
from
78a8d26 to
9fe9e4a
Compare
Initial Testinfra integration tests for AMI builds.
TODO: