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

bugfix: Filter rs that are not part of the current Deployement #191

Merged
merged 1 commit into from
Dec 26, 2023

Conversation

zhengjr9
Copy link
Contributor

Ⅰ. Describe what this PR does

If the selectors of multiple deployments are the same, subsequent tuner operations will be performed to rs that is not the current deployment, resulting in frequent scale-up and scale -down releases

@kruise-bot
Copy link

Welcome @zhengjr9! It looks like this is your first PR to openkruise/rollouts 🎉

@codecov-commenter
Copy link

codecov-commenter commented Dec 20, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (8620408) 43.39% compared to head (4dcbc42) 43.40%.
Report is 1 commits behind head on master.

Files Patch % Lines
pkg/controller/deployment/deployment_controller.go 55.55% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #191   +/-   ##
=======================================
  Coverage   43.39%   43.40%           
=======================================
  Files          52       52           
  Lines        5666     5674    +8     
=======================================
+ Hits         2459     2463    +4     
- Misses       2784     2786    +2     
- Partials      423      425    +2     
Flag Coverage Δ
unittests 43.40% <55.55%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@veophi
Copy link
Member

veophi commented Dec 21, 2023

@zhengjr9 Have you met this problem ever?
Theoretically, these miss-owned ReplicaSets will be claimed by the native deployment controller, so we don't add this logic.

@veophi
Copy link
Member

veophi commented Dec 21, 2023

The DCO failed. Please git commit --amend -s to signoff yourself and force push again.

@zhengjr9
Copy link
Contributor Author

Reference in new

yes,if i had 2 deploy which they has the same selector in the same namespace,the rollout controller while scale down and up another deploy that i didn't update it.that is the logs when i didn't update this deploy but it have relate a rollout.

urceVersion:"2864418754", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set test-debug-1-6db77f98b6 to 2 from 1
I1225 07:02:37.337774       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418754", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-service-message-converged-service-75776fddbc to 2 from 1
I1225 07:02:37.344387       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418754", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-service-system-atomic-service-b599b5644 to 2 from 1
I1225 07:02:37.349581       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418754", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-operations-dev-85884b9f77 to 2 from 1
I1225 07:02:37.356030       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418754", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-service-order-converged-service-7686bc9fd9 to 2 from 1
I1225 07:02:37.367093       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418754", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-service-user-converged-service-689658f675 to 2 from 1
I1225 07:02:37.372413       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418754", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-service-application-atomic-service-6586746865 to 2 from 1
I1225 07:02:37.622697       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418831", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-test-rppll-695d87b49c to 0 from 3
I1225 07:02:37.630095       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418831", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set iecom-ebi-finance-5c6777f8dd to 2 from 1
I1225 07:02:37.645174       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418831", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-service-enterprise-atomic-service-66c4b5fb5b to 2 from 1
I1225 07:02:38.089751       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set dev-mbpm-etl-85c7bc8f48 to 0 from 2
I1225 07:02:38.194954       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-t-oss-77cf6bb898 to 2 from 1
I1225 07:02:38.531822       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set bard-agent-94c944776 to 0 from 2
I1225 07:02:38.538863       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-svc-demo-796f44f796 to 3 from 2
I1225 07:02:38.549307       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set apollo-agent-595fbfbdb6 to 2 from 1
I1225 07:02:38.558517       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-sentinel-dashboard-dev-85b966f496 to 2 from 1
I1225 07:02:38.589784       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-eureka-service-dev-02-774d8898b4 to 4 from 1
I1225 07:02:38.596439       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set iecom-ebi-export-v1-0-9b985dddf to 2 from 1
I1225 07:02:38.614378       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-file-server-dev-77995c8cc9 to 2 from 1
I1225 07:02:38.629823       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-service-message-atomic-service-6b74544cf to 2 from 1
I1225 07:02:38.637471       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-base-eureka-devloper-58b859dfd8 to 2 from 1
I1225 07:02:38.643121       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-dev-gw-config-77bf676bc7 to 2 from 1
I1225 07:02:38.654720       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-paas-eureka1-dev-8db8558fc to 2 from 1
I1225 07:02:38.665317       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set deployment-iecom-ebi-etl-749dbbb57 to 2 from 1
I1225 07:02:38.672323       1 event.go:282] Event(v1.ObjectReference{Kind:"Deployment", Namespace:"default", Name:"test-mtu-2", UID:"093623c8-13e6-44fb-b3ee-1da52d95d0c3", APIVersion:"apps/v1", ResourceVersion:"2864418854", FieldPath:""}): type: 'Normal' reason: 'ScalingReplicaSet' Scaled down replica set iam-data-sync-prd-dev-74dc9646c6 to 2 from 1

// select rs owner by current deployment
ownedRSs := make([]*apps.ReplicaSet, 0)
for _, rs := range allRSs {
if rs.DeletionTimestamp.IsZero() {
Copy link
Member

Choose a reason for hiding this comment

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

if !rs.DeletionTimestamp.IsZero() {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry,it was a silly mistake

@zhengjr9 zhengjr9 force-pushed the bugfix/select_owner_rs branch 2 times, most recently from f867f18 to 48c4c8f Compare December 26, 2023 03:09
@veophi
Copy link
Member

veophi commented Dec 26, 2023

The DCO failed. Please git commit --amend -s to signoff yourself and force push again.

@zhengjr9

@veophi
Copy link
Member

veophi commented Dec 26, 2023

/lgtm

@veophi
Copy link
Member

veophi commented Dec 26, 2023

/approve

@kruise-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: veophi

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 1e84129 into openkruise:master Dec 26, 2023
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants