Skip to content

Draft: [CI/CD] Build docker container and deploy to kube#1

Open
zinstack625 wants to merge 5 commits intoFe-Ti:masterfrom
zinstack625:master
Open

Draft: [CI/CD] Build docker container and deploy to kube#1
zinstack625 wants to merge 5 commits intoFe-Ti:masterfrom
zinstack625:master

Conversation

@zinstack625
Copy link

This needs 3 secrets:

  • DOCKERHUB_USERNAME
  • DOCKERHUB_TOKEN
  • KUBECONFIG

Before being ready for merge, container name needs changing. I don't know how it will be called, so this is blocked for now

@Fe-Ti
Copy link
Owner

Fe-Ti commented Sep 22, 2023

container name needs changing

Maybe it's better to use a CI variable/secret as a container name?

Copy link
Owner

@Fe-Ti Fe-Ti left a comment

Choose a reason for hiding this comment

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

Except for my concerns about using variables, everything looks nice

build:
uses: ./.github/workflows/build-image.yml
with:
image_name: zinstack625/iscraweb-ttbot
Copy link
Owner

Choose a reason for hiding this comment

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

E.g. here could be something like

      image_name: ${{ secrets.DOCKERHUB_USERNAME }}/${{ vars.CONTAINER_NAME }}

command: |
echo -n "$KUBECONFIG_FILE" > .kubeconfig
export KUBECONFIG=$(pwd)/.kubeconfig
kubectl rollout restart -n iscrawebtt deployment ttbot-bleed
Copy link
Owner

@Fe-Ti Fe-Ti Sep 22, 2023

Choose a reason for hiding this comment

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

Same goes for line 26:

kubectl rollout restart -n $NAMESPACE $RESOURCE_TYPE $RESOURCE

@@ -0,0 +1,12 @@
#!/usr/bin/env sh

[ -z "$CONTAINER_NAME" ] && CONTAINER_NAME=docker.io/Fe-Ti/iscraweb-ttbot:latest
Copy link
Owner

@Fe-Ti Fe-Ti Sep 22, 2023

Choose a reason for hiding this comment

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

Maybe it's better to stop execution with error instead of hardcoding a link*:

die() {
    echo "$1"; exit 1
}
[ -z "$CONTAINER_NAME" ] && die 'CONTAINER_NAME is not set! Set envvar $CONTAINER_NAME'

  • Fun note: I've tried to create account with dash, but docker hub said that it's prohibited :-)

@Fe-Ti
Copy link
Owner

Fe-Ti commented Sep 24, 2023

I've made some changes to underlying library, so parameter notify_period was replaced with optional notify_schedule.

Also with recent changes¹ there is new command called notify, which triggers notification procedure. That in turn makes it possible to control notifications via cron.


¹ Now bot uses http and json if started with -s key. Input format is:

{
    "commands" : ["cmd",...] | "cmd"
}

{
"refresh_period" : $REFRESH_PERIOD,
"sleep_timeout" : $SLEEP_TIMEOUT,
"notify_period" : $NOTIFY_PERIOD,
Copy link
Owner

@Fe-Ti Fe-Ti Sep 28, 2023

Choose a reason for hiding this comment

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

This line (4) should be removed if we're going to rely on cron (I suppose it's better).

Or it should look like this (if we rely on internal implementation):

    "notify_schedule"       : ["09:00", "21:00"],

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants