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

Resolve all filepaths to absolute in 'skaffold diagnose' #5791

Merged

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented May 5, 2021

Description
this change forces absolute path resolution for all paths provided in a skaffold.yaml when running skaffold diagnose.

User facing changes (remove if N/A)
paths provided in a skaffold.yaml will now always be resolved to absolute.

given our getting-started example:

Before:

➜ skaffold diagnose --yaml-only
apiVersion: skaffold/v2beta16
kind: Config
build:
  artifacts:
  - image: skaffold-example
    context: .
    docker:
      dockerfile: Dockerfile
  tagPolicy:
    gitCommit: {}
  local:
    concurrency: 1
deploy:
  kubectl:
    manifests:
    - k8s-*
  logs:
    prefix: container

After:

➜ skaffold diagnose --yaml-only
apiVersion: skaffold/v2beta16
kind: Config
build:
  artifacts:
  - image: skaffold-example
    context: /Users/nkubala/Development/skaffold/examples/getting-started
    docker:
      dockerfile: Dockerfile
  tagPolicy:
    gitCommit: {}
  local:
    concurrency: 1
deploy:
  kubectl:
    manifests:
    - /Users/nkubala/Development/skaffold/examples/getting-started/k8s-*
  logs:
    prefix: container

the purpose of skaffold diagnose is to inspect exactly what skaffold is doing at runtime - this change makes which files skaffold is using more clear. additionally, this makes the output of skaffold diagnose --yaml-only more useful in successive skaffold runs outside of the root directory (specifically, providing it to skaffold render).

@nkubala nkubala requested a review from a team as a code owner May 5, 2021 19:23
@nkubala nkubala requested a review from tejal29 May 5, 2021 19:23
@google-cla google-cla bot added the cla: yes label May 5, 2021
@nkubala nkubala enabled auto-merge (squash) May 5, 2021 19:24
@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #5791 (e41b3fb) into master (d14f34d) will increase coverage by 0.00%.
The diff coverage is 40.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #5791    +/-   ##
========================================
  Coverage   70.78%   70.79%            
========================================
  Files         432      436     +4     
  Lines       16242    16357   +115     
========================================
+ Hits        11497    11580    +83     
- Misses       3901     3923    +22     
- Partials      844      854    +10     
Impacted Files Coverage Δ
pkg/skaffold/build/jib/init.go 83.05% <0.00%> (-4.45%) ⬇️
pkg/skaffold/build/jib/jib.go 69.46% <0.00%> (-1.08%) ⬇️
pkg/skaffold/config/options.go 100.00% <ø> (ø)
pkg/skaffold/parser/config.go 79.56% <ø> (-0.13%) ⬇️
pkg/skaffold/build/jib/jvm.go 55.55% <55.55%> (ø)
cmd/skaffold/app/cmd/diagnose.go 64.51% <100.00%> (+1.18%) ⬆️
pkg/skaffold/color/formatter.go 96.00% <0.00%> (-4.00%) ⬇️
pkg/skaffold/docker/image.go 78.34% <0.00%> (-1.39%) ⬇️
pkg/skaffold/schema/profiles.go 88.46% <0.00%> (-1.15%) ⬇️
cmd/skaffold/skaffold.go 0.00% <0.00%> (ø)
... and 13 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 9a89e8f...e41b3fb. Read the comment docs.

@nkubala nkubala merged commit 8406ce1 into GoogleContainerTools:master May 5, 2021
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.

2 participants