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 patch for UnitedDeployment #1266

Merged
merged 1 commit into from
Jun 5, 2023
Merged

Conversation

chengleqi
Copy link
Contributor

@chengleqi chengleqi commented Apr 24, 2023

Ⅰ. Describe what this PR does

Add logic related to StrategicMergePatch in ApplySubsetTemplate method in each Adapter (AdvancedStatefulSetAdapter | CloneSetAdapter | DeploymentAdapter | StatefulSetAdapter).

Ⅱ. Does this pull request fix one issue?

#1265

Ⅲ. Describe how to verify it

Use following sample:

apiVersion: apps.kruise.io/v1alpha1
kind: UnitedDeployment
metadata:
  labels:
    controller-tools.k8s.io: "1.0"
  name: uniteddeployment-sample
spec:
  replicas: 6
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app: foo
  template:
    statefulSetTemplate:
      metadata:
        labels:
          app: foo
      spec:
        template:
          metadata:
            labels:
              app: foo
          spec:
            containers:
              - name: nginx
                image: nginx
  topology:
    subsets:
      - name: subset-a
        replicas: 1
        patch:
            spec:
              containers:
                - name: nginx
                  image: nginx:1.23.3-alpine
        nodeSelectorTerm:
          matchExpressions:
            - key: node
              operator: In
              values:
                - zone-a
      - name: subset-b
        replicas: 50%
        nodeSelectorTerm:
          matchExpressions:
            - key: node
              operator: In
              values:
                - zone-b
      - name: subset-c
        nodeSelectorTerm:
          matchExpressions:
            - key: node
              operator: In
              values:
                - zone-c

Ⅳ. Special notes for reviews

@kruise-bot kruise-bot requested review from Fei-Guo and furykerry April 24, 2023 13:57
@kruise-bot kruise-bot added the size/L size/L: 100-499 label Apr 24, 2023
@@ -18,7 +18,11 @@

import (
"context"
"encoding/json"
"fmt"
Copy link

Choose a reason for hiding this comment

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

38% of developers fix this issue

goimports: File is not goimports-ed

❗❗ 2 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
pkg/controller/uniteddeployment/adapter/cloneset_adapter.go 6
pkg/controller/uniteddeployment/adapter/deployment_adapter.go 22

Visit the Lift Web Console to find more details in your report.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -18,7 +18,10 @@

import (
"context"
"encoding/json"
"fmt"
Copy link

Choose a reason for hiding this comment

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

38% of developers fix this issue

goimports: File is not goimports-ed


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@codecov-commenter
Copy link

codecov-commenter commented May 7, 2023

Codecov Report

Merging #1266 (ac236cc) into master (7d134bf) will decrease coverage by 0.16%.
The diff coverage is 24.61%.

❗ 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.

@@            Coverage Diff             @@
##           master    #1266      +/-   ##
==========================================
- Coverage   48.60%   48.44%   -0.16%     
==========================================
  Files         151      151              
  Lines       21005    21113     +108     
==========================================
+ Hits        10210    10229      +19     
- Misses       9680     9769      +89     
  Partials     1115     1115              
Flag Coverage Δ
unittests 48.44% <24.61%> (-0.16%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ontroller/daemonset/daemonset_predownload_image.go 0.00% <0.00%> (ø)
pkg/controller/sidecarset/sidecarset_processor.go 67.78% <0.00%> (-0.83%) ⬇️
...oller/statefulset/statefulset_predownload_image.go 0.00% <0.00%> (ø)
...deployment/adapter/advanced_statefulset_adapter.go 0.00% <0.00%> (ø)
...oller/uniteddeployment/adapter/cloneset_adapter.go 0.00% <0.00%> (ø)
...ler/uniteddeployment/adapter/deployment_adapter.go 0.00% <0.00%> (ø)
...er/uniteddeployment/adapter/statefulset_adapter.go 0.00% <0.00%> (ø)
.../controller/cloneset/cloneset_predownload_image.go 62.80% <88.88%> (+1.94%) ⬆️
...er/uniteddeployment/uniteddeployment_controller.go 72.56% <100.00%> (+1.19%) ⬆️
...roller/uniteddeployment/uniteddeployment_update.go 74.78% <100.00%> (+0.22%) ⬆️

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

@chengleqi chengleqi closed this May 16, 2023
@chengleqi chengleqi reopened this May 16, 2023
@chengleqi chengleqi marked this pull request as ready for review May 16, 2023 14:37
@kruise-bot kruise-bot requested review from FillZpp and zmberg May 16, 2023 14:37
@veophi

This comment was marked as duplicate.

@@ -146,6 +150,27 @@ func (a *DeploymentAdapter) ApplySubsetTemplate(ud *alpha1.UnitedDeployment, sub
attachNodeAffinity(&set.Spec.Template.Spec, subSetConfig)
attachTolerations(&set.Spec.Template.Spec, subSetConfig)

if subSetConfig.Patch.Raw != nil {
Copy link
Member

Choose a reason for hiding this comment

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

plz add ut that contain patches

@kruise-bot kruise-bot added size/XL size/XL: 500-999 and removed size/L size/L: 100-499 labels May 23, 2023
@chengleqi chengleqi force-pushed the add-patch branch 2 times, most recently from 1cb6a08 to ac236cc Compare May 29, 2023 15:21
Signed-off-by: chengleqi <chengleqi5g@hotmail.com>

generate manifests and fix goimports

Signed-off-by: chengleqi <chengleqi5g@hotmail.com>

fix goimports

Signed-off-by: chengleqi <chengleqi5g@hotmail.com>

patch to PodTemplateSpec

Signed-off-by: chengleqi <chengleqi5g@hotmail.com>

add ut for uniteddeployment patch

Signed-off-by: chengleqi <chengleqi5g@hotmail.com>
Copy link
Member

@furykerry furykerry left a comment

Choose a reason for hiding this comment

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

/lgtm

@zmberg
Copy link
Member

zmberg commented Jun 5, 2023

/approve

@kruise-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zmberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kruise-bot kruise-bot merged commit 6fda363 into openkruise:master Jun 5, 2023
@chengleqi chengleqi deleted the add-patch branch June 5, 2023 17:19
diannaowa pushed a commit to diannaowa/kruise that referenced this pull request Aug 29, 2023
generate manifests and fix goimports



fix goimports



patch to PodTemplateSpec



add ut for uniteddeployment patch

Signed-off-by: chengleqi <chengleqi5g@hotmail.com>
lilongfeng0902 pushed a commit to lilongfeng0902/kruise that referenced this pull request Sep 12, 2023
generate manifests and fix goimports



fix goimports



patch to PodTemplateSpec



add ut for uniteddeployment patch

Signed-off-by: chengleqi <chengleqi5g@hotmail.com>
ppbits pushed a commit to ppbits/kruise that referenced this pull request Apr 4, 2024
generate manifests and fix goimports



fix goimports



patch to PodTemplateSpec



add ut for uniteddeployment patch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants