-
Notifications
You must be signed in to change notification settings - Fork 20
fix: check if tsuru.yaml has process commands before fallbacking to i… #46
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
fix: check if tsuru.yaml has process commands before fallbacking to i… #46
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds logic to prefer processes defined in tsuru.yaml over falling back to a platform Procfile/command, and introduces tests and testdata for this behavior.
- Parse tsuru.yaml to detect defined processes and only fallback when both Procfile and tsuru.yaml processes are absent.
- Add end-to-end test and Python app test data to validate tsuru.yaml hooks, processes, package installs, and Python version.
- Switch YAML dependency to sigs.k8s.io/yaml and update Go/toolchain and several module versions.
Reviewed Changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/build/buildkit/build.go | Adds tsuru.yaml process check, swaps YAML lib, adjusts permissions notation. |
| pkg/build/buildkit/build_test.go | Renames existing test and adds new integration test for user-defined tsuru.yaml. |
| pkg/build/buildkit/testdata/python-tsuru-yaml/tsuru.yaml | Provides tsuru.yaml with build hooks and a web process for testing. |
| pkg/build/buildkit/testdata/python-tsuru-yaml/requirements.txt | Python requirements for the test app. |
| pkg/build/buildkit/testdata/python-tsuru-yaml/requirements.apt | System packages for the test app. |
| pkg/build/buildkit/testdata/python-tsuru-yaml/app.py | Minimal Flask app used in tests. |
| pkg/build/buildkit/testdata/python-tsuru-yaml/hook.py | Auxiliary test script. |
| go.mod | Updates Go/toolchain and dependencies, adds tsuru/types and sigs.k8s.io/yaml. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Hi @ravilock This change covers the case where we have a tsuru.yaml file defined with processes, and the process does not have a start command. processes:
- name: web
# Using the command from the Dockerfile
command: npm start
healthcheck:
path: /health
scheme: http
allowed_failures: 3
force_restart: true
interval_seconds: 60
timeout_seconds: 60
deploy_timeout_seconds: 600 |
- Replaced usage of github.com/tsuru/tsuru types with local TsuruYamlData and related structs in pkg/build/types.go. - Refactored buildkit build logic to use new tsuru.yaml parsing and process handling. - Updated tests and testdata to support multiple processes and new tsuru.yaml structure. - Added misc/local-dev.sh script for local loopback IP setup/cleanup. - Improved Makefile with setup/cleanup targets and Docker Compose detection. - Removed tsuru dependency from go.mod/go.sum and added new indirect dependencies.
d59ea3a to
e3f0a12
Compare
crgisch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Would be nice to add on README how to run/test locally using new Makefile commands :)
…mage command