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

Remove associated NLBs with MCIS del #1201

Merged
merged 1 commit into from
Oct 13, 2022
Merged

Conversation

seokho-son
Copy link
Member

  • MCIS 삭제 시, 내부에 연관된 NLB도 모두 삭제하도록 자원 정리 자동화.
  • MCIS 삭제 결과에 대한 세부 정보 제공.

Request URL

http://localhost:1323/tumblebug/ns/ns01/mcis/mcis01

Response body

{
  "output": [
    "VM: group-0-1 [Done]",
    "VM: group-0-2 [Done]",
    "VM: group-0-3 [Done]",
    "vmGroup: group-0 [Done]",
    "NLB: group-0 [Done]",
    "MCIS: mcis01 [Done]"
  ]
}

@lgtm-com
Copy link

lgtm-com bot commented Oct 13, 2022

This pull request introduces 1 alert when merging 588554e into efd7b4c - view on LGTM.com

new alerts:

  • 1 for Log entries created from user input

Copy link
Member

@jihoon-seo jihoon-seo left a comment

Choose a reason for hiding this comment

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

LGTM 입니다. 😊

Comment on lines 1751 to 1755
err = common.CBStore.Delete(vmKey)
if err != nil {
common.CBLog.Error(err)
return err
return deletedResources, err
}
Copy link
Member

Choose a reason for hiding this comment

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

'vmList 의 vm들을 하나씩 지우는 for 문' 을 돌다가
특정 vm 삭제에 실패하면
return deletedResources, err 하는 것이 아니라
deletedResources.IdList = append(deletedResources.IdList, "VM: "+v+" [Failed]") 하는 것이 어떨까 싶기도 했었는데

(option=force 경우가 아닌 일반적인 경우에는)
DelMcis 함수는 Running VM이 있는 경우에는 삭제를 수행하지 않고
Terminated / Failed 등의 VM이 있는 경우에만 삭제를 수행하며
이 삭제 동작이 내부적으로 수행하는 것은 CBStore.Delete 인데
이전 코드들을 고려하면 이 동작이 실패할 확률은 거의 없어서
현재 로직도 괜찮을 것 같습니다. 😊

Comment on lines +1798 to 1802
output, err := DelAllNLB(nsId, mcisId, "", forceFlag)
if err != nil {
common.CBLog.Error(err)
return deletedResources, err
}
Copy link
Member

Choose a reason for hiding this comment

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

이 부분에서, 예를 들어 MCIS 내에 3개의 NLB가 있었는데
DelAllNLB 실행 결과 2개는 삭제 성공, 1개는 삭제 실패 이면
DelAllNLB의 리턴값인 output, err

output:

[
"NLB: nlb01 [Done]",
"NLB: nlb02 [Done]",
"NLB: nlb03 <Some error>"
]

err: nil
로 오며

이후 L1803부터 다시 진행되어
K-V store에서 MCIS 객체를 삭제합니다.
(즉, MCIS 내에 있었던 3개의 NLB 중 하나는 삭제되지 않은 상태로 MCIS가 삭제됨)

저는 이것(↑)도 괜찮은 것 같습니다.. ㅎㅎ
(NLB 삭제를 시도했지만 실패한 것이니.. 사용자에게 출력해주고, proceed하여 나머지 object 삭제)

@jihoon-seo jihoon-seo merged commit a63144d into cloud-barista:main Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants