-
Notifications
You must be signed in to change notification settings - Fork 378
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
gc namespace for k8sspark k8sflink #3392
gc namespace for k8sspark k8sflink #3392
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3392 +/- ##
==========================================
+ Coverage 18.00% 18.02% +0.02%
==========================================
Files 1403 1403
Lines 145380 145392 +12
==========================================
+ Hits 26171 26212 +41
+ Misses 116512 116480 -32
- Partials 2697 2700 +3
|
0c0d676
to
4c429a6
Compare
modules/pipeline/conf/conf.go
Outdated
@@ -112,6 +112,9 @@ type Conf struct { | |||
// scheduler executor refresh interval | |||
ExecutorRefreshIntervalMinute uint64 `env:"EXECUTOR_REFRESH_INTERVAL_MINUTE" default:"20"` | |||
SpecifyImagePullPolicy string `env:"SPECIFY_IMAGE_PULL_POLICY" default:"IfNotPresent"` | |||
|
|||
// k8s namespace | |||
EnableSpecifiedK8SNamespace string `env:"ENABLE_SPECIFIED_K8S_NAMESPACE"` |
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 think specified_k8s_namespace
should be a request level config.
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.
adjusted, only use task.Extra.NotPipelineControlledNs
NotPipelineControlledNs is request level config
modules/pipeline/conf/conf.go
Outdated
@@ -112,6 +112,9 @@ type Conf struct { | |||
// scheduler executor refresh interval | |||
ExecutorRefreshIntervalMinute uint64 `env:"EXECUTOR_REFRESH_INTERVAL_MINUTE" default:"20"` | |||
SpecifyImagePullPolicy string `env:"SPECIFY_IMAGE_PULL_POLICY" default:"IfNotPresent"` | |||
|
|||
// k8s namespace | |||
EnableSpecifiedK8SNamespace string `env:"ENABLE_SPECIFIED_K8S_NAMESPACE"` |
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.
use bool
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.
who inject this env
|
||
// delete namespace after gc flinkcluster | ||
namespace := task.Extra.Namespace | ||
if conf.EnableSpecifiedK8SNamespace() == "" && !task.Extra.NotPipelineControlledNs { |
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.
bad string comparation.
4c429a6
to
d1d562c
Compare
/approve |
What type of this PR
Add one of the following kinds:
/kind feature
What this PR does / why we need it:
gc namespace for k8sspark k8sflink
Which issue(s) this PR fixes:
Specified Reviewers:
/assign @your-reviewer
ChangeLog
Need cherry-pick to release versions?
Add comment like
/cherry-pick release/1.0
when this PR is merged.