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

build: replace kit with task #11928

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

build: replace kit with task #11928

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
75ebacc
build: replace `kit` with `task`
agilgur5 Oct 2, 2023
1e9d3c5
build: rename `tasks.yaml` -> `Taskfile.yml`
agilgur5 Oct 2, 2023
7937cbb
chore: follow Taskfile styleguide
agilgur5 Oct 2, 2023
3040183
build: replace all installation and usage of `kit` with `task`
agilgur5 Oct 2, 2023
6aaf99e
add task checksums to gitignore
agilgur5 Oct 3, 2023
bf07cea
add back `./hack/port-forward.sh` to `ci-build.yaml`
agilgur5 Oct 3, 2023
26d51fe
properly background continuous tasks
agilgur5 Oct 3, 2023
8a1c81f
optimize `ui-deps` task to not rerun unnecessarily
agilgur5 Oct 3, 2023
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
6 changes: 3 additions & 3 deletions .devcontainer/pre-build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ chmod +x ./kubectl
sudo mv ./kubectl /usr/local/bin/kubectl
kubectl cluster-info

# install kit
curl -q https://raw.githubusercontent.com/kitproj/kit/main/install.sh | sh
# install task
sh -c "$(curl --location https://taskfile.dev/install.sh)" -- -d -b /usr/local/bin

# install protocol buffer compiler (protoc)
sudo apt update
Expand All @@ -23,7 +23,7 @@ sudo apt install -y protobuf-compiler
sudo chown -R vscode:vscode /home/vscode/go

# download dependencies and do first-pass compile
CI=1 kit pre-up
CI=1 task pre-up

# Patch CoreDNS to have host.docker.internal inside the cluster available
kubectl get cm coredns -n kube-system -o yaml | sed "s/ NodeHosts: |/ NodeHosts: |\n `grep host.docker.internal /etc/hosts`/" | kubectl apply -f -
4 changes: 3 additions & 1 deletion .github/workflows/ci-build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,12 @@ jobs:
- name: Install manifests
run: make install PROFILE=${{matrix.profile}} STATIC_FILES=false
- name: Build controller
run: make controller kit STATIC_FILES=false
run: make controller task STATIC_FILES=false
- name: Build CLI
run: make cli STATIC_FILES=false
if: ${{matrix.test == 'test-api' || matrix.test == 'test-cli' || matrix.test == 'test-java-sdk' || matrix.test == 'test-python-sdk'}}
- name: Start port forward
run: ./hack/port-forward.sh
Comment on lines +156 to +157
Copy link
Member Author

@agilgur5 agilgur5 Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

this was removed in the goreman -> kit migration (this line), added it back now since task does not handle port forwarding

Copy link
Member Author

@agilgur5 agilgur5 Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

this fixed the Controller E2E tests. some Server tests are failing because apparently the Server didn't start at all. apparently task was waiting for deps to finish (which totally makes sense), but starting a servers will never finish. so the cmds need to be backgrounded properly. then everything works ok.

I think kit's ports keyword (mentioned above) was just watching for ports to be opened before proceeding to the next task. nifty feature for servers.

- name: Start controller/API
run: make start PROFILE=${{matrix.profile}} AUTH_MODE=client STATIC_FILES=false LOG_LEVEL=info API=${{matrix.test == 'test-api' || matrix.test == 'test-cli' || matrix.test == 'test-java-sdk' || matrix.test == 'test-python-sdk'}} UI=false > /tmp/argo.log 2>&1 &
- name: Wait for controller to be up
Expand Down
4 changes: 3 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ Pipfile
.idea/
.node-version
.DS_Store
# checksums etc that `task` stores for caching / memoizing
.task/
vendor/
dist/
# delve debug binaries
Expand Down Expand Up @@ -45,6 +47,6 @@ sdks/python/client/dist/*
manifests/install.yaml
manifests/namespace-install.yaml
/logs
node_modules
node_modules
result
.devenv
23 changes: 9 additions & 14 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -517,27 +517,22 @@ dist/argosay:
mkdir -p dist
cp test/e2e/images/argosay/v2/argosay dist/

.PHONY: kit
kit:
ifeq ($(shell command -v kit),)
ifeq ($(shell uname),Darwin)
brew tap kitproj/kit --custom-remote https://github.com/kitproj/kit
brew install kit
else
curl -q https://raw.githubusercontent.com/kitproj/kit/main/install.sh | tag=v0.1.8 sh
endif
.PHONY: task
task:
# update this in Nix when upgrading it here
ifneq ($(USE_NIX), true)
npm i -g @go-task/[email protected]
Comment on lines +520 to +524
Copy link
Member

Choose a reason for hiding this comment

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

Needs updating in the Nix file, happy to do that myself if you want?
Technically the Nix setup doesn't need task though because it already has something similar.

Copy link
Member Author

@agilgur5 agilgur5 Oct 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@isubasinghe could you elaborate on this? You mentioned devenv before, but I didn't know that had task runner support (again, my Nix knowledge is pretty dated)?

We discussed it at the Contributor meeting too if it could be used as a replacement on all environments, but no one knew the answer so we were looking to you for more info.
There was also this warning in the Nix docs that we weren't sure exactly what it applied to

Copy link
Member Author

Choose a reason for hiding this comment

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

Checked it myself and it looks like devenv up is a replacement for goreman, but neither are full task runners, they just run processes concurrently (no deps, sources, env, etc).

task and kit are make replacements rather than goreman replacements.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah it does have a task runner, infact Nix can replace the Makefile and task/goreman/etc.
https://github.com/argoproj/argo-workflows/blob/master/dev/nix/flake.nix#L393-L403 is how the processes to run are declared.

There is a manual step of setting up the k8s cluster needed at the moment but even this can be fully integrated into Nix.

That controller should have a version attached to it, I did not set this version. So production builds using Nix are not really possible yet. This is still very fixable though, more than happy to talk about it if this is something we want to do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it does have a task runner, infact Nix can replace the Makefile and task/goreman/etc.
https://github.com/argoproj/argo-workflows/blob/master/dev/nix/flake.nix#L393-L403 is how the processes to run are declared.

Yea I've seen that -- that's equivalent to goreman but that doesn't have deps, sources, env etc -- features of task, kit, and make. Unless that feature exists somewhere too and just isn't being used?

endif


.PHONY: start
ifeq ($(RUN_MODE),local)
ifeq ($(API),true)
start: install controller kit cli
start: install controller task cli
else
start: install controller kit
start: install controller task
endif
else
start: install kit
start: install task
endif
@echo "starting STATIC_FILES=$(STATIC_FILES) (DEV_BRANCH=$(DEV_BRANCH), GIT_BRANCH=$(GIT_BRANCH)), AUTH_MODE=$(AUTH_MODE), RUN_MODE=$(RUN_MODE), MANAGED_NAMESPACE=$(MANAGED_NAMESPACE)"
ifneq ($(API),true)
Expand All @@ -558,7 +553,7 @@ endif
grep '127.0.0.1.*postgres' /etc/hosts
grep '127.0.0.1.*mysql' /etc/hosts
ifeq ($(RUN_MODE),local)
env DEFAULT_REQUEUE_TIME=$(DEFAULT_REQUEUE_TIME) ARGO_SECURE=$(SECURE) ALWAYS_OFFLOAD_NODE_STATUS=$(ALWAYS_OFFLOAD_NODE_STATUS) ARGO_LOG_LEVEL=$(LOG_LEVEL) UPPERIO_DB_DEBUG=$(UPPERIO_DB_DEBUG) ARGO_AUTH_MODE=$(AUTH_MODE) ARGO_NAMESPACED=$(NAMESPACED) ARGO_NAMESPACE=$(KUBE_NAMESPACE) ARGO_MANAGED_NAMESPACE=$(MANAGED_NAMESPACE) ARGO_EXECUTOR_PLUGINS=$(PLUGINS) PROFILE=$(PROFILE) kit $(TASKS)
env DEFAULT_REQUEUE_TIME=$(DEFAULT_REQUEUE_TIME) ARGO_SECURE=$(SECURE) ALWAYS_OFFLOAD_NODE_STATUS=$(ALWAYS_OFFLOAD_NODE_STATUS) ARGO_LOG_LEVEL=$(LOG_LEVEL) UPPERIO_DB_DEBUG=$(UPPERIO_DB_DEBUG) ARGO_AUTH_MODE=$(AUTH_MODE) ARGO_NAMESPACED=$(NAMESPACED) ARGO_NAMESPACE=$(KUBE_NAMESPACE) ARGO_MANAGED_NAMESPACE=$(MANAGED_NAMESPACE) ARGO_EXECUTOR_PLUGINS=$(PLUGINS) PROFILE=$(PROFILE) task $(TASKS)
endif

.PHONY: wait
Expand Down
109 changes: 109 additions & 0 deletions Taskfile.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# yaml-language-server: $schema: https://taskfile.dev/schema.json
Copy link
Member Author

@agilgur5 agilgur5 Oct 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

the schema is a pretty nice addition too 😉

it's got integrations with VSCode and other editors as well if that's your preference


# Install `task` by following https://taskfile.dev/installation/
# Run `task up` to start argo.

# - `env PROFILE=mysql task up` to start with MySQL.
# - `env PROFILE=plugins ARGO_EXECUTOR_PLUGINS=true task up` to start with plugins.
# - `env PROFILE=sso ARGO_AUTH_MODE=sso task up` to start with SSO.

# The app will be up-and-running between 15s and 1m later (if hot compiled or cold).
# When using `--watch`, any changes made to the source code will be automatically recompiled and the app restarted, typically within a few seconds.

version: 3

tasks:
go-deps:
cmd: go mod download
# mutex: download
Copy link
Member Author

@agilgur5 agilgur5 Oct 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think any of these mutexes are actually strictly necessary, two downloads and two builds can run in parallel, they just may bottleneck (or they may not, I don't believe they do on my laptop), so it might be a concurrency optimization to run them serially instead.

Task does have the run key to ensure something runs only once. And it does have a serial syntax, but it's not as flexible as a string for a mutex lock (not entirely the same semantics as a mutex does not specify ordering)

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed the download mutex in #13350


install:
cmd: sh -c "make install PROFILE=$PROFILE"
deps: [go-deps]
sources: [manifests]
env:
PROFILE: minimal
# mutex: docker

build-controller:
cmd: make ./dist/workflow-controller
deps: [go-deps]
sources: [cmd/workflow-controller, config, errors, persist, pkg, util, workflow]
# mutex: build

port-forward:
cmd: ./hack/port-forward.sh
deps: [install]
# ports: 9000
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved

controller:
cmd: ./dist/workflow-controller
deps: [install, build-controller, port-forward]
env:
ARGO_EXECUTOR_PLUGINS: false
ARGO_NAMESPACE: argo
ARGO_NAMESPACED: true
ARGO_MANAGED_NAMESPACE: argo
ARGO_LOG_LEVEL: info
ARGO_REMOVE_PVC_PROTECTION_FINALIZER: true
ARGO_PROGRESS_PATCH_TICK_DURATION: 7s
DEFAULT_REQUEUE_TIME: 1s
LEADER_ELECTION_IDENTITY: local
ALWAYS_OFFLOAD_NODE_STATUS: false
OFFLOAD_NODE_STATUS_TTL: 30s
WORKFLOW_GC_PERIOD: 30s
UPPERIO_DB_DEBUG: 1
ARCHIVED_WORKFLOW_GC_PERIOD: 30s
# ports: "9090"

build-argo:
cmd: make ./dist/argo
deps: [go-deps]
sources: [cmd/argo, config, errors, persist, pkg, util, server, workflow]
env:
STATIC_FILES: false
# mutex: build

server:
cmd: ./dist/argo server
deps: [build-argo, port-forward]
env:
ARGO_X_FRAME_OPTIONS: SAMEORIGIN
ARGO_SECURE: false
ARGO_NAMESPACE: argo
ARGO_NAMESPACED: true
ARGO_LOG_LEVEL: info
ARGO_AUTH_MODE: hybrid
ARGO_MANAGED_NAMESPACE: argo
UPPERIO_DB_DEBUG: 1
# ports: "2746"

ui-deps:
cmd: yarn install
dir: ui
# mutex: downloads

ui:
cmd: yarn start
dir: ui
deps: [ui-deps, server]
# ports: "8080"

executor:
cmd: make argoexec-image
sources: [cmd/argoexec, config, errors, pkg, util, workflow]
# mutex: docker

example:
cmd: kubectl create -f examples/hello-world.yaml
deps: [install]
# mutex: docker

build:
deps: [build-controller, build-argo]

pre-up:
deps: [build, install, executor, example]

up:
deps: [pre-up, controller, server, ui]
99 changes: 0 additions & 99 deletions tasks.yaml
View file
Open in desktop

This file was deleted.

Loading