Updates to Docker execution

[emmacasolin]

Description

#432 is making changes to the way we do process execution, which impacts our docker tests. This requires updating the affected tests, redeploying in the CI/CD, and sanity checks for the expected behaviour.

Related to https://gitlab.com/MatrixAI/Engineering/Polykey/Polykey-Infrastructure/-/merge_requests/1

Issues Fixed

• Relates Standardise Process Execution in tests/utils/exec.ts #432

Tasks

• 1. Change release.nix to use Entrypoint instead of Cmd
• 2. Update the task definition in Polykey-Infrastructure
• 3. Update all docker tests
• 4. Merge https://gitlab.com/MatrixAI/Engineering/Polykey/Polykey-Infrastructure/-/merge_requests/1
• 5. Redeploy container image and service
• 6. Sanity checks for signalling and process events
• 7. Update README instructions for container/docker usage

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

• Make big changes easier — review code in small groups of related files

• Know where to start — see the whole change at a glance

• Take a code tour — explore the change with an interactive tour

• Make comments and review — all fully sync’ed with github

  Try it now!

Legend

[emmacasolin]

This is the PR in Polykey-Infrastructure for updating the task definition: https://gitlab.com/MatrixAI/Engineering/Polykey/Polykey-Infrastructure/-/merge_requests/1

[emmacasolin]

One of the biggest changes coming in from #436 is that pkStdio is no longer compatible with docker integration tests. This is because the original pkStdio was not compatible with these tests since it mocked stdout and stderr. It was changed to essentially just call pkExec underneath when used with docker, however, this functionality is being removed. Any tests that need to run on docker need to switch from using pkStdio to using pkExec, a change which is being made to existing tests in #436 so this just needs to be kept in mind for any future tests we want to add to our suite of docker integration tests.

[CMCDragonkai]

One of the biggest changes coming in from #436 is that pkStdio is no longer compatible with docker integration tests. This is because the original pkStdio was not compatible with these tests since it mocked stdout and stderr. It was changed to essentially just call pkExec underneath when used with docker, however, this functionality is being removed. Any tests that need to run on docker need to switch from using pkStdio to using pkExec, a change which is being made to existing tests in #436 so this just needs to be kept in mind for any future tests we want to add to our suite of docker integration tests.

This is correct. The pkStdio should not be used unless the test is expected to never be an integration test. So any test using pkStdio should be convert to pkExec in this scenario. However with the loss of the ability to mock STDIO we need pkExpect to be shell capable so we can do proper stdio expectations. Please link that issue and that will be a subsequent work to do, which may involve forking nexpect.

[CMCDragonkai]

However we are not replacing the pkStdio tests for now. That's assigned to MatrixAI/Polykey-CLI#7.

This should only be affecting existing docker tests by removing the /bin/polykey.

[CMCDragonkai]

@tegefaulkes can you point out where we need to remove /bin/polykey when docker integration tests are run?

[tegefaulkes]

/bin/polykey needs to be removed from the PK_TEST_COMMAND in the gitlab ci file. at .gitlab-ci.yml:342 PK_TEST_COMMAND="docker run \$DOCKER_OPTIONS $image_and_tag /bin/polykey" npm run test -- tests/bin;

[emmacasolin]

PK_TEST_COMMAND has been updated. This should now be ready for CI/CD redeployment.

[CMCDragonkai]

Are the recent job failures the same timeouts as before? Not introduced by this PR?

[CMCDragonkai]

Also have you see that the Nat tests are timing out. Can this be fixed up in your testnet tests PR?

[emmacasolin]

The failures are all timeouts on tests that usually take very close to the set timeout time to complete, so if they take slightly too long they timeout. They can't be due to this PR since this PR only touches the integration:docker job and release.nix, neither of which would affect tests during the check stage.

There's only one NAT test timing out, and it usually takes close to 40s to run. It doesn't always timeout on the CI/CD but when it does it's most likely just that test taking slightly too long. The only fix would be increasing the timeout or waiting to see if #419 causes the test to run any faster.

[CMCDragonkai]

Merged on Polykey-Infrastructure. This requires an image build and upload. Image is uploaded here, so until CI/CD is green we have to do this manually.

[CMCDragonkai]

Manual deployment done and instructions updated: 7b1edbd

taskDefinition: 'arn:aws:ecs:ap-southeast-2:015248367786:task-definition/polykey-testnet:76',

[CMCDragonkai]

I've updated the task definition on AWS. And I noticed that the image itself doesn't have a HOME which impacts the NODE_PATH automatic derivation.

If a NODE_PATH and HOME doesn't exist, it ends up defaulting to /.local/share/polykey (because os.homeDir() becomes just /).

This is something to be aware of. I also fixed up the path construction to be using path.join, so the help page now shows:

  -np, --node-path <path>      Path to Node State (default: "/.local/share/polykey", env: PK_NODE_PATH)

This should also help MatrixAI/Polykey-CLI#14 and MatrixAI/Polykey-CLI#11

[CMCDragonkai]

Service redeployed.

  {
      "serviceName": "polykey-testnet",
      "serviceArn": "arn:aws:ecs:ap-southeast-2:015248367786:service/polykey-testnet/polykey-testnet",
      "status": "ACTIVE",
      "deployments": [
          {
              "id": "ecs-svc/9149761156517349517",
              "status": "PRIMARY",
              "rolloutState": "IN_PROGRESS",
              "rolloutStateReason": "ECS deployment ecs-svc/9149761156517349517 in progress."
          },
          {
              "id": "ecs-svc/6135588724958369979",
              "status": "ACTIVE",
              "rolloutState": "IN_PROGRESS",
              "rolloutStateReason": "ECS deployment ecs-svc/6135588724958369979 in progress."
          },
          {
              "id": "ecs-svc/1428782724658529208",
              "status": "ACTIVE",
              "rolloutState": "COMPLETED",
              "rolloutStateReason": "ECS deployment ecs-svc/1428782724658529208 completed."
          }
      ]
  }

Now we wait if the new task definition continues to work. @emmacasolin please check in with AWS dashboard and logs tomorrow as you're writing testnet tests to see if it is deployed correctly. The command is now set to just set to:

            "command": [
                "agent",
                "start",
                "--verbose",
                "--format=json"
            ],

And it should be appending to the entrypoint being /bin/polykey.

Furthermore commands are changed to:

docker run -it polykey-1.0.0:rkl4ma7wbfvsz1wl95gn9rpz865gk2ai --help

@tegefaulkes remember to check in to see if docker integration tests are still working as well when you rebase. Specifically in reference to 6..