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

fix l4lb delete lvs services unexpectedly #59

Merged
merged 3 commits into from Jul 13, 2018
Merged

fix l4lb delete lvs services unexpectedly #59

merged 3 commits into from Jul 13, 2018

Conversation

mainred
Copy link
Contributor

@mainred mainred commented Jul 12, 2018

JIRA: https://jira.mesosphere.com/browse/DCOS_OSS-3602

l4lb manager has a mistake when caculating services to deleted and
and to added.

for example, we have 1,2,4,5 vips originally, and new vip 3 is added,
so we have the following execution according to function generate_diff

seq oldvips newvips toAdd toDelete Mutation
1 [1,2,4,5] [1,2,3,4,5]
2 [2,4,5] [2,3,4,5] [] [] []
3 [4,5] [3,4,5] [] [] []
4 [5] [3,4,5] [] [4] []
5 [] [3,4,5] [] [4,5] []
return [3,4,5] [4,5] []

l4lb manager has a mistake when caculating services to deleted and
and to added.

for example, we have 1,2,4,5 vips originally, and new vip 3 is added,
so we have the following execution according to function generate_diff

    oldvips         newvips          toAdd    toDelete    Mutation
1. [1,2,4,5]      [1,2,3,4,5]
2. [2,4,5]         [2,3,4,5]            []          []         []
3. [4,5]            [3,4,5]             []          []         []
4. [5]               [3,4,5]            []          [4]        []
5. []                 [3,4,5]           []         [4,5]       []

return                               [3,4,5]        [4,5]        []
@mesosphere-ci
Copy link

Can one of the admins verify this patch?

@codecov
Copy link

codecov bot commented Jul 12, 2018

Codecov Report

Merging #59 into master will increase coverage by 0.06%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
+ Coverage   67.96%   68.03%   +0.06%     
==========================================
  Files          48       48              
  Lines        2897     2900       +3     
==========================================
+ Hits         1969     1973       +4     
+ Misses        928      927       -1
Impacted Files Coverage Δ
apps/dcos_l4lb/src/dcos_l4lb_mgr.erl 70.11% <100%> (+1.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 03d5b4d...e6fcc60. Read the comment docs.

@urbanserj urbanserj changed the base branch from master to 1.11.x July 12, 2018 16:48
@urbanserj urbanserj changed the base branch from 1.11.x to master July 12, 2018 23:53
@urbanserj
Copy link
Contributor

hey @mainred, thank you so much for the patch.

Please ignore my previous message, I've deleted it.

Your patch looks great, good catch! Could you please just check-pick my commit and rebase your branch to current master?

Copy link
Contributor

@urbanserj urbanserj left a comment

Choose a reason for hiding this comment

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

LGTM

@urbanserj urbanserj merged commit 2e22c50 into dcos:master Jul 13, 2018
@urbanserj
Copy link
Contributor

@mainred thank you very much for your contribution!

@urbanserj urbanserj mentioned this pull request Jul 17, 2018
7 tasks
@mainred mainred deleted the fix_delete_service branch May 22, 2019 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants