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

add option to pass arguments to kustomize, fixes #2488 #2700

Closed
wants to merge 3 commits into from
Closed

add option to pass arguments to kustomize, fixes #2488 #2700

wants to merge 3 commits into from

Conversation

daddz
Copy link
Contributor

@daddz daddz commented Aug 22, 2019

Related issue: #2488

@daddz
Copy link
Contributor Author

daddz commented Aug 22, 2019

What is the preferred format here?

deploy:
  kustomize:
    build:
        - "--load_restrictor none"

or

deploy:
  kustomize:
    build:
        - "--load_restrictor"
        - "none"

I went with the first option for now but could easily change to option 2.

Copy link
Contributor

@nkubala nkubala left a comment

Choose a reason for hiding this comment

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

What is the preferred format here?

both of those formats will be supported by this change. I think splitting on spaces is probably fine, though this is kind of a weird way to specify args IMO (--load_restrictor=none seems easier to reason about to me)

docs/content/en/schemas/v1beta13.json Outdated Show resolved Hide resolved
pkg/skaffold/schema/latest/config.go Outdated Show resolved Hide resolved
@daddz
Copy link
Contributor Author

daddz commented Aug 29, 2019

@nkubala, thanks for the feedback! I addressed your comments.
I've seen there's been a freeze for v1beta13. Shall I rebase and move these changes to v1beta14?

@tejal29
Copy link
Contributor

tejal29 commented Sep 10, 2019

@daddz,
Can you please do the following break this change in 2 changes ?

  1. Add BuildArgs field to KustomizeDeployer struct

    • Add a function buildCommandArgs which refactors L251 to L269
        def buildCommandArgs(args []string) []args {
      
      
         } 
      
    • Add unit tests
  2. Finally make a change to skaffold config KustomizeDeploy and add an example in integration/examples/kustomize or some documentation on how to use it?

@daddz
Copy link
Contributor Author

daddz commented Sep 11, 2019

@tejal29 Sure thing, I'll look into it tomorrow/friday.

@tejal29
Copy link
Contributor

tejal29 commented Sep 16, 2019

This is now merged as #2871 and #2870

@tejal29 tejal29 closed this Sep 16, 2019
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.

5 participants