Refactor workflows to add deployment of contracts migrated on Celo #2492

michalinacienciala · 2021-06-10T11:52:19Z

Workflows have been refactored so that Celo-related artifacts could be
deployed in the chain of automatic execusions triggered by the manual
dispatch of the Main workflow in the ci project (with environment
set to alfajores).

Similar changes have been introduced to keep-core and tbtc
workflows. See:
keep-network/keep-ecdsa#823
keep-network/tbtc#804

Workflows have been refactored so that Celo-related artifacts could be deployed in the chain of automatic execusions triggered by the manual dispatch of the `Main` workflow in the `ci` project (with `environment` set to `alfajores`).

.github/workflows/client.yml

nkuba · 2021-06-11T09:34:41Z

solidity/truffle-config.js

@@ -48,7 +48,7 @@ module.exports = {

    alfajores: {
      provider: function () {
-        const kit = Kit.newKit("https://alfajores-forno.celo-testnet.org")
+        const kit = Kit.newKit(process.env.CELO_HOSTNAME)


Maybe we should expect the same-named env variables for all networks.

So instead of having ETH_HOSTNAME and CELO_HOSTNAME we could expect CHAIN_API_URL,
and instead of CONTRACT_OWNER_ETH_ACCOUNT_PRIVATE_KEY and CONTRACT_OWNER_CELO_ACCOUNT_PRIVATE_KEY we could expect CONTRACT_OWNER_ACCOUNT_PRIVATE_KEY.

This will make the migrations more generic, hence from the CI workflows perspective, we could handle both Ethereum and Celo migrations in one step.

I did unified the variables (see 76adcf0), but I'm not sure if it will allow us to handle both migrations in one step (see my comment below).

I was also thinking about loading secrets for the environemnt based on the environment property. So we would name the GH environments the same as we expect workflow dispatch environment property to be set.

The downside of this solution would be the need to repeat the same settings across different repositories. Sticking with current solution seems to be the least complicated.

nkuba · 2021-06-11T09:37:09Z

.github/workflows/contracts.yml

@@ -247,7 +243,7 @@ jobs:
      - name: Build and publish initcontainer
        uses: docker/build-push-action@v2
        env:
-          IMAGE_NAME: 'initcontainer-provision-keep-client'
+          IMAGE_NAME: initcontainer-provision-keep-client-${{ github.event.inputs.environment }}


Renaming the image will require aligning the kubernetes configs.

Are those configs stored on the repo? Could you guide me where to find them?

Please grep ocurrences under ./infrastructure/kube/keep-dev and ./infrastructure/kube/keep-test.

I've changed the Kubernetes config and also modified the image name in the workflow to use chain name in the suffix instead of the environment/network. In order for this to work, keep-network/ci#6 needs to be merged to master, so please merge that PR before merging the current one.

nkuba · 2021-06-11T09:38:44Z

.github/workflows/contracts.yml

+      - name: Migrate contracts on Ethereum
+        if: github.event.inputs.environment != 'alfajores'
+        env:
          ETH_HOSTNAME: ${{ secrets.KEEP_TEST_ETH_HOSTNAME }}
          CONTRACT_OWNER_ETH_ACCOUNT_PRIVATE_KEY: |
            ${{ secrets.KEEP_TEST_ETH_CONTRACT_OWNER_PRIVATE_KEY }}
        run: npx truffle migrate --reset --network $TRUFFLE_NETWORK
+
+      - name: Migrate contracts on Celo
+        if: github.event.inputs.environment == 'alfajores'
+        env:
+          CELO_HOSTNAME: ${{ secrets.KEEP_TEST_CELO_HOSTNAME }}
+          CONTRACT_OWNER_CELO_ACCOUNT_PRIVATE_KEY: |
+            ${{ secrets.KEEP_TEST_CELO_CONTRACT_OWNER_PRIVATE_KEY }}
+        run: npx truffle migrate --reset --network $TRUFFLE_NETWORK


We could handle these two in a one step if we update env variables names expected by the truffle configuration (see the other comment).

My understanding is that if we'd switch to setting CHAIN_API_URL and CONTRACT_OWNER_ACCOUNT_PRIVATE_KEY environment variables, we would still need to populate those variables with different values, depending on the network used. Is there a way to do that in one step?
Maybe there is a way to do something like this?:

CHAIN_API_URL: if github.event.inputs.environment != 'alfajores' then set ${{ secrets.KEEP_TEST_ETH_HOSTNAME }}; else set ${{ secrets.KEEP_TEST_CELO_HOSTNAME }};

(I don't know how to do that with operators supported by GitHub)

I'm afraid it's not yet supported by GHA 😞
See: actions/runner#409

Name of the environment variables for different networks have been unified to make the migrations more generic.

Inintcontainer image name will now contain a suffix reflecting the chain for which it is configured. In the future we may add a suffix with the network name as well.

This was referenced Jun 10, 2021

Refactor workflows to add deployment of contracts migrated on Celo keep-network/keep-ecdsa#823

Merged

Refactor workflows to add deployment of contracts migrated on Celo keep-network/tbtc#804

Merged

michalinacienciala force-pushed the rfc-18/celo branch from 1323bbc to 3578a8d Compare June 10, 2021 13:27

michalinacienciala marked this pull request as ready for review June 10, 2021 14:18

michalinacienciala requested a review from nkuba June 10, 2021 14:19

nkuba requested changes Jun 11, 2021

View reviewed changes

Unification of env variables names

76adcf0

Name of the environment variables for different networks have been unified to make the migrations more generic.

michalinacienciala requested a review from nkuba June 11, 2021 11:36

michalinacienciala added 3 commits June 11, 2021 13:45

Merge remote-tracking branch 'origin/master' into rfc-18/celo

2d969b3

Switch order of steps in the workflow config to improve readibility

c0b590d

Reference the chain in the initcontainer image name

f5bb7d0

Inintcontainer image name will now contain a suffix reflecting the chain for which it is configured. In the future we may add a suffix with the network name as well.

nkuba approved these changes Jul 2, 2021

View reviewed changes

nkuba merged commit 8a53319 into master Jul 2, 2021

nkuba deleted the rfc-18/celo branch July 2, 2021 10:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor workflows to add deployment of contracts migrated on Celo #2492

Refactor workflows to add deployment of contracts migrated on Celo #2492

michalinacienciala commented Jun 10, 2021 •

edited

Loading

nkuba Jun 11, 2021

michalinacienciala Jun 11, 2021

nkuba Jun 14, 2021

michalinacienciala Jun 16, 2021

nkuba Jun 11, 2021

michalinacienciala Jun 11, 2021

nkuba Jun 14, 2021

michalinacienciala Jun 16, 2021 •

edited

Loading

nkuba Jun 11, 2021

michalinacienciala Jun 11, 2021

nkuba Jun 14, 2021

Refactor workflows to add deployment of contracts migrated on Celo #2492

Refactor workflows to add deployment of contracts migrated on Celo #2492

Conversation

michalinacienciala commented Jun 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalinacienciala Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalinacienciala commented Jun 10, 2021 •

edited

Loading

michalinacienciala Jun 16, 2021 •

edited

Loading