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.

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

Implement teardown action #51

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

stevefan1999-personal
Copy link

This should clean-up the k3s cluster running (if any), so that https://github.com/nektos/act can be used repeatedly.

@kuritka
Copy link
Collaborator

kuritka commented Dec 4, 2023

Hi @stevefan1999-personal, I don't understand why to delete k3d clusters in post-event if the whole workflow run will be deleted after it finishes.

run.sh Outdated
Comment on lines 76 to 87
local name=${CLUSTER_NAME}
local arguments=${ARGS:-}
local k3dVersion=${K3D_VERSION:-${DEFAULT_K3D_VERSION}}

if [[ -z "${CLUSTER_NAME}" ]]; then
panic "CLUSTER_NAME must be set"
fi

echo -e "${YELLOW}Downloading ${CYAN}k3d@${k3dVersion} ${NC}see: ${K3D_URL}"
curl --silent --fail ${K3D_URL} | TAG=${k3dVersion} bash

echo -e "\existing_network${YELLOW}Teardown cluster ${CYAN}$name ${NC}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if there is a need to re-download the k3d binary that is already there.

Choose a reason for hiding this comment

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

Well, at least for some reason it was needed for https://github.com/nektos/act

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you mentioned kind action in a related issue. Teardown also does not install kind again.

Copy link
Collaborator

@kuritka kuritka left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, it's an excellent idea and I don't mind adding this feature, but I'd like GitHub events to run in GitHub as before. I guess there's no reason to download k3d again on post or use 3rd party action inside ours.

Please, squash your commits.

We have tests in .github/workflows. If teardown could be set explicitly, it would be good to see evidence of it running as it should.

In the case of explicit execution , extra documentation will be needed.

Comment on lines +36 to +41
- uses: gacts/run-and-post-run@v1
with:
run: |
${{ github.action_path }}/run.sh deploy
post: |
${{ github.action_path }}/run.sh teardown
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm afraid that this would be an unnecessary way to run teardown over and over again in VM-based workflows. Could it be done differently?

Choose a reason for hiding this comment

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

Well, I think you shouldn't assume it will be VM based since you can run act locally -- given you just have Docker access, and act pretty much replicates 60% of GHA to let you run it locally so that you can test the workflow before it goes live on master.

run.sh Outdated
Comment on lines 76 to 87
local name=${CLUSTER_NAME}
local arguments=${ARGS:-}
local k3dVersion=${K3D_VERSION:-${DEFAULT_K3D_VERSION}}

if [[ -z "${CLUSTER_NAME}" ]]; then
panic "CLUSTER_NAME must be set"
fi

echo -e "${YELLOW}Downloading ${CYAN}k3d@${k3dVersion} ${NC}see: ${K3D_URL}"
curl --silent --fail ${K3D_URL} | TAG=${k3dVersion} bash

echo -e "\existing_network${YELLOW}Teardown cluster ${CYAN}$name ${NC}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you mentioned kind action in a related issue. Teardown also does not install kind again.

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