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

Clarify how Skaffold resolves files #5467

Merged
merged 2 commits into from
Mar 17, 2021

Conversation

briandealwis
Copy link
Member

Related: #5355

Description
Clarify where the skaffold.yaml is expected to be found and how referenced files and directories are resolved against the current working directory, except for build artifacts.

@briandealwis briandealwis requested a review from a team as a code owner February 26, 2021 22:45
@briandealwis briandealwis added docs-modifications runs the docs preview service on the given PR and removed size/M labels Feb 26, 2021
@google-cla google-cla bot added the cla: yes label Feb 26, 2021
@container-tools-bot
Copy link

Please visit http://35.236.91.127:1313 to view changes to the docs.

@container-tools-bot container-tools-bot removed the docs-modifications runs the docs preview service on the given PR label Feb 26, 2021
@container-tools-bot
Copy link

Error creating deployment docs-controller-deployment-5467, please visit https://storage.googleapis.com/webhook-logs/logs-5467-1614379720231026047 to view logs.

@container-tools-bot
Copy link

Error creating deployment, please see controller logs for details.

@codecov
Copy link

codecov bot commented Feb 26, 2021

Codecov Report

Merging #5467 (04b2aca) into master (827139d) will decrease coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5467      +/-   ##
==========================================
- Coverage   71.48%   71.28%   -0.21%     
==========================================
  Files         397      400       +3     
  Lines       14573    14975     +402     
==========================================
+ Hits        10418    10675     +257     
- Misses       3387     3512     +125     
- Partials      768      788      +20     
Impacted Files Coverage Δ
pkg/skaffold/test/test_factory.go 72.09% <0.00%> (-6.86%) ⬇️
pkg/skaffold/runner/new.go 63.13% <0.00%> (-4.46%) ⬇️
pkg/skaffold/instrumentation/meter.go 54.54% <0.00%> (-3.52%) ⬇️
cmd/skaffold/app/cmd/diagnose.go 33.33% <0.00%> (-1.45%) ⬇️
pkg/skaffold/docker/image.go 78.13% <0.00%> (-1.40%) ⬇️
pkg/skaffold/deploy/helm/deploy.go 75.48% <0.00%> (-0.50%) ⬇️
pkg/skaffold/kubernetes/watcher.go 84.37% <0.00%> (-0.48%) ⬇️
...ffold/kubernetes/portforward/resource_forwarder.go 85.71% <0.00%> (-0.29%) ⬇️
pkg/skaffold/event/event.go 90.78% <0.00%> (ø)
cmd/skaffold/app/cmd/build.go 89.74% <0.00%> (ø)
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 827139d...04b2aca. Read the comment docs.

build:
artifacts:
- image: app
context: frontend
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 mention that if context is not explicitly defined then it's set to the CWD anyway?

is resolved relative to the `frontend` directory (i.e., `frontend/Dockerfile`),
whereas the the Helm chart's location and its values-files are
relative to the current directory in `helm/project`.
```yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

also show the directory tree?

.
├── frontend
│   └── Dockerfile
├── helm
│   └── project
│       └── dev-values.yaml
└── skaffold.yaml

`skaffold.yaml` in the current directory, but the location can be
overridden with the `--filename` flag.

### File resolution
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a File resolution section down below or mention here itself about configurations resolved as dependencies. The paths in those files are resolved relative to the directory containing the imported file. So it's recommended to have the skaffold.yaml file live in the root of the project and not in a nested directory.

Copy link
Member Author

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

Oh jeepers, I missed your comments @gsquared94. PTAL.

Copy link
Contributor

@gsquared94 gsquared94 left a comment

Choose a reason for hiding this comment

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

lgtm!

@briandealwis briandealwis merged commit cd25008 into GoogleContainerTools:master Mar 17, 2021
@briandealwis briandealwis deleted the fileresolving branch March 17, 2021 11:12
@briandealwis
Copy link
Member Author

Failure was due to Docker Hubtoomanyrequests

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

Successfully merging this pull request may close these issues.

3 participants