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

feat(kraken): push to kube-manifests repository #147

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

feat(kraken): push to kube-manifests repository #147

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6782cb2
feat(kraken): push to kube-manifests repository
jonathankao97 Apr 4, 2022
01ad54b
Merge branch 'master' into kraken
jonathankao97 Apr 16, 2022
0547c72
fix: add var name
jonathankao97 Apr 16, 2022
bf486bf
fix: temporarily add back eks deploy
jonathankao97 Apr 16, 2022
34bd9af
chore: update snapshots
jonathankao97 Apr 16, 2022
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
37 changes: 22 additions & 15 deletions cdk/kraken/src/deploy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,20 @@ export class DeployJob extends CheckoutJob {
runsOn: 'ubuntu-latest',
if: `github.ref == 'refs/heads/${fullConfig.defaultBranch}'`,
steps: [
{
name: 'Checkout kube-manifests',
uses: 'actions/checkout@v2',
with: {
repository: 'pennlabs/kube-manifests',
token: '${{ secrets.BOT_GITHUB_PAT }}',
path: 'kube-manifests',
}
},
{
name: 'Configure git',
run: dedent`git config --global user.name github-actions
git config --global user.email github-actions[bot]@users.noreply.github.com"`
},
{
id: 'synth',
name: 'Synth cdk8s manifests',
Expand All @@ -60,21 +74,14 @@ export class DeployJob extends CheckoutJob {
},
},
{
name: 'Deploy',
run: dedent`aws eks --region us-east-1 update-kubeconfig --name production --role-arn arn:aws:iam::\${AWS_ACCOUNT_ID}:role/kubectl

# get repo name from synth step
RELEASE_NAME=\${{ steps.synth.outputs.RELEASE_NAME }}
Copy link
Contributor

Choose a reason for hiding this comment

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

RELEASE_NAME=\${{ steps.synth.outputs.RELEASE_NAME }} is necessary to use the $RELEASE_NAME env variable in the git commit -m "chore(k8s): deploy $RELEASE_NAME" line you added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wups, thanks


# Deploy
kubectl apply -f k8s/dist/ -l app.kubernetes.io/component=certificate
kubectl apply -f k8s/dist/ --prune -l app.kubernetes.io/part-of=$RELEASE_NAME`,
env: {
AWS_ACCOUNT_ID: '${{ secrets.AWS_ACCOUNT_ID }}',
AWS_ACCESS_KEY_ID: '${{ secrets.GH_AWS_ACCESS_KEY_ID }}',
AWS_SECRET_ACCESS_KEY: '${{ secrets.GH_AWS_SECRET_ACCESS_KEY }}',
},
},
name: 'Push to kube-manifests repository',
Copy link
Contributor

Choose a reason for hiding this comment

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

We can push manifests to another repo, but we should still keep & run the deployment steps:

kubectl apply -f k8s/dist/ -l app.kubernetes.io/component=certificate
kubectl apply -f k8s/dist/ --prune -l app.kubernetes.io/part-of=$RELEASE_NAME`

Or did you plan for this to be ran as a part of the github actions for the kube-manifests repository? Might make more sense to keep deployments specific to products imo and just push to kube-manifests for record-keeping.

Also, can you update the snapshots?

Copy link
Member

Choose a reason for hiding this comment

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

The idea behind this PR is migrating to use argocd to manage our deployments rather than manually kubectl applying to the cluster. We can push yaml files to the kube-manifests repo, which argo is monitoring and it can then update our cluster using those changes.

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've added the eks deploy step back in for now. It's likely easier to deploy to kube-manifests and kubectl deploying while the Argo deployment is getting ironed out and then removing the kubectl deploy step afterwards.

Copy link
Member

Choose a reason for hiding this comment

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

Using both argo and the direct kubectl apply feels like we may end up with some weird race condition. It may make sense to remove the kubectl apply here,, finish the kittyhawk rollout, then migrate over to argo.

run: dedent`cd kube-manifests
mkdir -p \${{ github.repository }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make a directory with github.repository (so like a folder pennlabs then a subfolder penn-courses), or should we just make a directory with the RELEASE_NAME?

I feel like the latter might make more sense since everything will be prefixed by pennlabs anyways.

cp -r ../k8s/dist/ \${{ github.repository }}
git add \${{ github.repository }}
Copy link
Contributor

Choose a reason for hiding this comment

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

same question here as above ^^ (since we either change both or keep both)

git commit -m "chore(k8s): deploy $RELEASE_NAME"
Copy link
Member

Choose a reason for hiding this comment

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

Just a suggestion, but if we change the commit message to be something like "deploy pennlabs/repo-name@git-sha", we'll get a direct link to the commit that's being deployed. It might be a small QoL improvement.

git push`
}
],
...overrides,
});
Expand Down