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

[Feature-14421][K8S Task] Configurable image pull policy #14426

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

Radeity
Copy link
Member

@Radeity Radeity commented Jun 29, 2023

Purpose of the pull request

Brief change log

  • Support to select image pull policy from IfNotPresent, Always, Never.

Verify this pull request

image

@codecov-commenter
Copy link

Codecov Report

Merging #14426 (7d52487) into dev (5cbe170) will decrease coverage by 0.01%.
The diff coverage is 54.09%.

❗ Current head 7d52487 differs from pull request most recent head 00eb446. Consider uploading reports for the commit 00eb446 to get more accurate results

@@             Coverage Diff              @@
##                dev   #14426      +/-   ##
============================================
- Coverage     38.51%   38.51%   -0.01%     
+ Complexity     4566     4545      -21     
============================================
  Files          1237     1236       -1     
  Lines         43541    43410     -131     
  Branches       4813     4783      -30     
============================================
- Hits          16771    16720      -51     
+ Misses        24914    24832      -82     
- Partials       1856     1858       +2     
Impacted Files Coverage Δ
...scheduler/api/dto/resources/ResourceComponent.java 26.66% <ø> (+1.66%) ⬆️
...api/dto/resources/visitor/ResourceTreeVisitor.java 72.09% <ø> (-0.64%) ⬇️
...nscheduler/api/security/impl/ldap/LdapService.java 5.76% <0.00%> (-0.62%) ⬇️
...heduler/api/service/impl/ResourcesServiceImpl.java 44.53% <ø> (+0.67%) ⬆️
...cheduler/common/constants/DataSourceConstants.java 0.00% <ø> (ø)
...heduler/common/log/remote/OssRemoteLogHandler.java 0.00% <0.00%> (ø)
...apache/dolphinscheduler/dao/entity/DataSource.java 18.18% <ø> (ø)
...asource/api/datasource/BaseDataSourceParamDTO.java 40.42% <ø> (+4.57%) ⬆️
...nscheduler/service/process/ProcessServiceImpl.java 30.76% <ø> (-0.22%) ⬇️
.../org/apache/dolphinscheduler/spi/enums/DbType.java 0.00% <0.00%> (ø)
... and 19 more

... and 38 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Radeity Radeity self-assigned this Jun 29, 2023
@Radeity Radeity added feature new feature 3.2.0 for 3.2.0 version labels Jun 29, 2023
@Radeity Radeity added this to the 3.2.0 milestone Jun 29, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jun 29, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

93.8% 93.8% Coverage
0.0% 0.0% Duplication

Copy link
Member

@qingwli qingwli left a comment

Choose a reason for hiding this comment

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

We have defined Always and Never in here:

public static final String IMAGE_PULL_POLICY = "Always";
public static final String RESTART_POLICY = "Never";

Can we add a new constant IfNotPresent here and return a select list from API replace defined this in the UI module?

@Radeity
Copy link
Member Author

Radeity commented Jun 30, 2023

Can we add a new constant IfNotPresent here and return a select list from API replace defined this in the UI module?

Hi, @qingwli , thanks for your comment. I think image pull policies are relative static options, and it's still possible that user define a k8s task with wrong image pull policy by openAPI. In addition, we have already defined many option list in front-end codes, maybe for simplicity. Thus, considering interaction through UI page, do we have to make this change?

Copy link
Contributor

@rickchengx rickchengx left a comment

Choose a reason for hiding this comment

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

LGTM

@Radeity
Copy link
Member Author

Radeity commented Jul 4, 2023

Hi, @qingwli , do you have any other suggestions?

@qingwli
Copy link
Member

qingwli commented Jul 4, 2023

I agree with you @Radeity

@Radeity Radeity merged commit 65b6a4b into apache:dev Jul 5, 2023
IT-Kwj pushed a commit to IT-Kwj/dolphinscheduler that referenced this pull request Jul 14, 2023
zhongjiajie pushed a commit that referenced this pull request Jul 20, 2023
biaoma-ty pushed a commit to Kasma-Inc/dolphinscheduler that referenced this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.2.0 for 3.2.0 version backend document feature new feature UI ui and front end related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature][K8S Task] Configurable image pull policy
4 participants