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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 5 commits into from
Jul 2, 2021
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3578a8d
Refactor workflows to add deployment of contracts migrated on Celo
michalinacienciala Jun 10, 2021
76adcf0
Unification of env variables names
michalinacienciala Jun 11, 2021
2d969b3
Merge remote-tracking branch 'origin/master' into rfc-18/celo
michalinacienciala Jun 11, 2021
c0b590d
Switch order of steps in the workflow config to improve readibility
michalinacienciala Jun 16, 2021
f5bb7d0
Reference the chain in the initcontainer image name
michalinacienciala Jun 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions .github/workflows/client.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,11 @@ jobs:

- name: Load environment variables
uses: keep-network/load-env-variables@v1
if: github.event_name == 'workflow_dispatch'
env:
CI_GITHUB_TOKEN: ${{ secrets.CI_GITHUB_TOKEN }}
with:
# TODO: Consider passing of `environment` input instead of using
# hardcoded value. Would require some rework in action's code or
# in config files.
environment: 'ropsten'
environment: ${{ github.event.inputs.environment }}

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v1
Expand Down Expand Up @@ -99,7 +97,17 @@ jobs:
username: _json_key
password: ${{ secrets.KEEP_TEST_GCR_JSON_KEY }}

- name: Build Docker Runtime Image
nkuba marked this conversation as resolved.
Show resolved Hide resolved
if: github.event_name != 'workflow_dispatch'
uses: docker/build-push-action@v2
env:
IMAGE_NAME: 'keep-client'
with:
labels: revision=${{ github.sha }}
push: false

- name: Build and publish Docker Runtime Image
if: github.event_name == 'workflow_dispatch'
uses: docker/build-push-action@v2
env:
IMAGE_NAME: 'keep-client'
Expand All @@ -109,7 +117,7 @@ jobs:
# We don't use TAG yet, will be added at later stages of work on RFC-18.
tags: ${{ env.GCR_REGISTRY_URL }}/${{ env.GOOGLE_PROJECT_ID }}/${{ env.IMAGE_NAME }}
labels: revision=${{ github.sha }}
push: ${{ github.event_name == 'workflow_dispatch' }}
push: true

- name: Notify CI about completion of the workflow
if: github.event_name == 'workflow_dispatch'
Expand Down
111 changes: 24 additions & 87 deletions .github/workflows/contracts.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,10 @@ jobs:
- name: Lint
run: npm run lint

contracts-migrate-and-publish-ethereum:
contracts-migrate-and-publish:
needs: [contracts-build-and-test]
if: github.event_name == 'workflow_dispatch'
environment: keep-test # line can be deleted once we'll no longer use environment protection
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [12.x]
defaults:
run:
working-directory: ./solidity
Expand All @@ -131,15 +127,11 @@ jobs:
env:
CI_GITHUB_TOKEN: ${{ secrets.CI_GITHUB_TOKEN }}
with:
# TODO: Consider passing of `environment` input instead of using
# hardcoded value. Would require some rework in action's code or
# in config files.
environment: 'ropsten'
environment: ${{ github.event.inputs.environment }}

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v2
- uses: actions/setup-node@v2
with:
node-version: ${{ matrix.node-version }}
node-version: "12.x"

- name: Cache node modules
uses: actions/cache@v2
Expand All @@ -156,12 +148,21 @@ jobs:
- name: Install dependencies
run: npm ci

- name: Migrate contracts
env:
- 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
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

Copy link
Member

Choose a reason for hiding this comment

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

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


- name: Copy artifacts
run: |
Expand All @@ -178,7 +179,8 @@ jobs:
commit: ${{ github.sha }}

- name: Push contracts to Tenderly
# TODO: once below action gets tagged replace `@main` with `@v1`
if: github.event.inputs.environment == 'ropsten'
# TODO: once below action gets tagged replace `@main` with `@v1`
uses: keep-network/tenderly-push-action@main
continue-on-error: true
with:
Expand All @@ -197,16 +199,13 @@ jobs:
- name: Upload keep-core contracts for initcontainer build
uses: actions/upload-artifact@v2
with:
name: Contracts (Node.js ${{ matrix.node-version }})
name: Contracts
path: ./solidity/build/contracts/*

contracts-build-and-publish-initcontainer:
needs: [contracts-migrate-and-publish-ethereum]
if: needs.contracts-migrate-and-publish-ethereum.result == 'success'
needs: [contracts-migrate-and-publish]
if: needs.contracts-migrate-and-publish.result == 'success'
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [12.x]
steps:
- uses: actions/checkout@v2

Expand All @@ -215,15 +214,12 @@ jobs:
env:
CI_GITHUB_TOKEN: ${{ secrets.CI_GITHUB_TOKEN }}
with:
# TODO: Consider passing of `environment` input instead of using
# hardcoded value. Would require some rework in action's code or
# in config files.
environment: 'ropsten'
environment: ${{ github.event.inputs.environment }}

- name: Download migrated contracts artifacts
uses: actions/download-artifact@v2
with:
name: Contracts (Node.js ${{ matrix.node-version }})
name: Contracts
path: ./infrastructure/kube/templates/keep-client/initcontainer/provision-keep-client/

- name: Set up Docker Buildx
Expand All @@ -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 }}
Copy link
Member

Choose a reason for hiding this comment

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

Renaming the image will require aligning the kubernetes configs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@michalinacienciala michalinacienciala Jun 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

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.

with:
context: ./infrastructure/kube/templates/keep-client/initcontainer/provision-keep-client/
# GCR image should be named according to following convention:
Expand All @@ -270,64 +266,5 @@ jobs:
environment: ${{ github.event.inputs.environment }}
upstream_builds: ${{ github.event.inputs.upstream_builds }}
upstream_ref: ${{ github.event.inputs.upstream_ref }}
version: ${{ needs.contracts-migrate-and-publish-ethereum.outputs.version }}

contracts-migrate-and-publish-celo:
needs: [contracts-build-and-test]
if: github.event_name == 'workflow_dispatch'
environment: keep-test # line can be deleted once we'll no longer use environment protection
runs-on: ubuntu-latest
defaults:
run:
working-directory: ./solidity
steps:
- uses: actions/checkout@v2

- name: Load environment variables
uses: keep-network/load-env-variables@v1
env:
CI_GITHUB_TOKEN: ${{ secrets.CI_GITHUB_TOKEN }}
with:
# TODO: Consider passing of `environment` input instead of using
# hardcoded value. Would require some rework in action's code or
# in config files.
environment: 'alfajores'

- uses: actions/setup-node@v2
with:
node-version: "12.x"
version: ${{ needs.contracts-migrate-and-publish.outputs.version }}

- name: Cache node modules
uses: actions/cache@v2
env:
cache-name: cache-solidity-node-modules
with:
path: ~/.npm # npm cache files are stored in `~/.npm` on Linux/macOS
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-build-${{ env.cache-name }}-
${{ runner.os }}-build-
${{ runner.os }}-

- name: Install dependencies
run: npm ci

- name: Migrate contracts
env:
CONTRACT_OWNER_CELO_ACCOUNT_PRIVATE_KEY: |
${{ secrets.KEEP_TEST_CELO_CONTRACT_OWNER_PRIVATE_KEY }}
run: npx truffle migrate --reset --network $TRUFFLE_NETWORK

# TODO: Add copy to `artifacts` dir and NPM publish steps once it's clear
# how artifacts should be tagged. Once that's done, no longer publish
# contracts to GC Bucket.

- uses: google-github-actions/[email protected]
with:
project_id: ${{ env.GOOGLE_PROJECT_ID }}
service_account_key: ${{ secrets.KEEP_TEST_GCR_JSON_KEY }}

- name: Upload contract data
run: |
cd build/contracts
gsutil -m cp * gs://${{ env.CONTRACT_DATA_BUCKET }}/keep-core-celo
2 changes: 1 addition & 1 deletion solidity/truffle-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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).

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

kit.addAccount(process.env.CONTRACT_OWNER_CELO_ACCOUNT_PRIVATE_KEY)
return kit.web3.currentProvider
},
Expand Down