feat: support merge cmd env with image extension #2207
Conversation
kakaZhou719
force-pushed
the
merge-cmd-env-with-image-extension
branch
from
9c77916
to
25a1dc2
Compare
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## main #2207 +/- ##
==========================================
- Coverage 19.93% 13.14% -6.80%
==========================================
Files 98 263 +165
Lines 9220 22696 +13476
==========================================
+ Hits 1838 2983 +1145
- Misses 7128 19308 +12180
- Partials 254 405 +151
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
kakaZhou719
force-pushed
the
merge-cmd-env-with-image-extension
branch
from
3439b80
to
cdb8e1f
Compare
| ) | ||
|
|
||
| // MergeClusterWithImageExtension :set default value get from image extension,such as image global env | ||
| func MergeClusterWithImageExtension(cluster *v2.Cluster, imageExt imagev1.ImageExtension) *v2.Cluster { | ||
| if len(imageExt.Env) > 0 { |
There was a problem hiding this comment.
I thought there will be another args to be merged later.
pkg/application/v2app.go
Outdated
| @@ -22,6 +22,8 @@ import ( | |||
| "strings" | |||
| "syscall" | |||
|
|
|||
| mapUtils "github.com/sealerio/sealer/utils/maps" | |||
|
|
|||
cmd/sealer/cmd/utils/application.go
Outdated
| @@ -20,7 +20,7 @@ import ( | |||
| ) | |||
|
|
|||
| //ConstructApplication merge flags to v2.Application | |||
| func ConstructApplication(app *v2.Application, cmds, appNames []string) *v2.Application { | |||
| func ConstructApplication(app *v2.Application, cmds, appNames, appEnvs []string) *v2.Application { | |||
There was a problem hiding this comment.
s/appEnvs/globalEnvs?
WDYT?
| // set merged cluster | ||
| cf.SetCluster(*cluster) | ||
| cf.SetCluster(*mergedWithExt) |
There was a problem hiding this comment.
I wonder if the Clusterfile saved to the kubernetes cluster will change?
There was a problem hiding this comment.
mergedWithExt is an expected value, it will not change the cluster which already saved.
Signed-off-by: kakzhou719 <kakazhou719@163.com> feat: support merge cmd env with image extension Signed-off-by: kakzhou719 <kakazhou719@163.com> feat:support env and appenv instruction
kakaZhou719
force-pushed
the
merge-cmd-env-with-image-extension
branch
from
9e65814
to
b6b4523
Compare
Describe what this PR does / why we need it
we have already supported set
ENVandAPPENVon Kubefile, each key-pairs will be saved in image extension.see : #2179.so , how to use the default env with the user-defined env, That's what this PR does.
example:
Kubefile:
ENVvalue to v2.ClusterSpec.Env, if not present, just append it.APPENVvalue to v2.ApplicationConfig.Env, if not present, just append it.priority order:
for cluster env:
user defined env from cmd flag
-e> v2.ClusterSpec.Env > ImageExtension.Envfor app env:
v2.ApplicationConfig.Env > v2.ClusterSpec.Env > ImageExtension.Applications.Env
Does this pull request fix one issue?
Describe how you did it
Describe how to verify it
Special notes for reviews