-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Support kubectl deploy absolute manifest files #2011
Support kubectl deploy absolute manifest files #2011
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Codecov Report
@@ Coverage Diff @@
## master #2011 +/- ##
==========================================
+ Coverage 55.96% 55.98% +0.01%
==========================================
Files 173 173
Lines 7556 7559 +3
==========================================
+ Hits 4229 4232 +3
Misses 2924 2924
Partials 403 403
Continue to review full report at Codecov.
|
df67ecf
to
2d63f6f
Compare
@tossmilestone before we review it, i want to understand more about your usecase. Thanks |
@tejal29 Yes, I am using one skaffold yaml for several projects located in different directory. So it's useful if I can deploy using absolute file. |
pkg/skaffold/util/util.go
Outdated
@@ -89,6 +89,12 @@ func StrSliceContains(sl []string, s string) bool { | |||
func ExpandPathsGlob(workingDir string, paths []string) ([]string, error) { | |||
expandedPaths := make(map[string]bool) | |||
for _, p := range paths { | |||
if _, err := os.Stat(p); err == nil { |
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.
Can you instead use AbsolutePaths
here?
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.
skaffold/pkg/skaffold/util/util.go
Line 265 in 782c111
func AbsolutePaths(workspace string, paths []string) []string { |
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.
OK
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.
I use filepath.IsAbs
instead, cause the AbsolutePaths
is used to prepend the relative paths.
Signed-off-by: Xiaoxi He <xxhe@alauda.io>
2d63f6f
to
4d81529
Compare
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 to me.
if filepath.IsAbs(p) { | ||
// This is a absolute file reference | ||
expandedPaths[p] = true | ||
continue |
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.
this short-circuits the glob expansion further below. Should it be that way?
Signed-off-by: Xiaoxi He xxhe@alauda.io
Fix #2010