Skip to content

Commit

Permalink
Fix the defer function in StartVM() to delete unassociated FloatingIP
Browse files Browse the repository at this point in the history
  • Loading branch information
@powerkimhub
powerkimhub committed Feb 7, 2025
1 parent 15e5bfc commit 9bd1392
Showing 1 changed file with 14 additions and 13 deletions.
27 changes: 14 additions & 13 deletions cloud-control-manager/cloud-driver/drivers/openstack/resources/VMHandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ import (
"encoding/json"
"errors"
"fmt"
"io"
"io/ioutil"
"math/rand"
"net"
"net/http"
"os"
"regexp"
"strconv"
"strings"
"time"

cdcom "github.com/cloud-barista/cb-spider/cloud-control-manager/cloud-driver/common"
"github.com/gophercloud/gophercloud"
volumes3 "github.com/gophercloud/gophercloud/openstack/blockstorage/v3/volumes"
Expand All @@ -26,16 +37,6 @@ import (
"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
layer3floatingips "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/floatingips"
"github.com/gophercloud/gophercloud/openstack/networking/v2/subnets"
"io"
"io/ioutil"
"math/rand"
"net"
"net/http"
"os"
"regexp"
"strconv"
"strings"
"time"

call "github.com/cloud-barista/cb-spider/cloud-control-manager/cloud-driver/call-log"
idrv "github.com/cloud-barista/cb-spider/cloud-control-manager/cloud-driver/interfaces"
Expand Down Expand Up @@ -249,17 +250,17 @@ func (vmHandler *OpenStackVMHandler) StartVM(vmReqInfo irs.VMReqInfo) (startvm i
}

var publicIPStr string
defer func(publicIP string) {
defer func() {
if createErr != nil {
cleanVMIId := irs.IID{
SystemId: server.ID,
}
cleanErr := vmHandler.vmCleaner(cleanVMIId, publicIP)
cleanErr := vmHandler.vmCleaner(cleanVMIId, publicIPStr)
if cleanErr != nil {
createErr = errors.New(fmt.Sprintf("%s Failed to rollback deleting err = %s", createErr, cleanErr))
}
}
}(publicIPStr)
}()

var serverResult *servers.Server
var serverInfo irs.VMInfo
Expand Down

3 comments on commit 9bd1392

@ish-hcc
Copy link
Member

@ish-hcc ish-hcc commented on 9bd1392 Feb 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@powerkimhub 변경하신 이유가 있으실까요? defer 함수에서 publicip 값이 변할 우려가 있지 않을까 해서 질문드립니다.

@powerkimhub
Copy link
Member Author

Choose a reason for hiding this comment

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

@ish-hcc



[추가 설명]

  • 시험해보니, 다음에서는 오류 상황에서 publicIP.FloatingIP는 IP를 잘 반환해주고 있습니다.
func (vmHandler *OpenStackVMHandler) AssociatePublicIP(serverID string) (bool, string, error) {
 ...

return false, publicIP.FloatingIP, errors.New(fmt.Sprintf("failed to associate floating ip to vm, exceeded maximum retry count %d, err = %s", maxRetryCnt, err),
		}
	}

  • 다음 코드에서도 StartVM() 내에서 접근 가능한 publicIPStr에 값을 잘 설정해주고 있습니다.
ok, ipStr, err := vmHandler.AssociatePublicIP(serverResult.ID)
			if !ok {
				publicIPStr = ipStr // 실패 시에도 생성된 public IP 값 반환됨

  • 위 PR Review 의견 드린 바와 같이, 기존 defer function에 정의된 변수가 포인터가 아니다 보니,
  • 넘겨지는publicIPStr 값과 관련 없이 local 변수로 새로 생성되어 전달이 안되고 항상 "" 이었습니다.
  • publicIPStr는 StartVM() 내에서 floating ip가 생성된 이후에는 항상 값이 유지 되니
  • defer 함수 내에 직접 활용하도록 했습니다.
  • 일단, 변경된 코드로 시험해 보니, Association 오류 시에도 floating ip가 잘 삭제되고 있음을 확인했습니다.

@ish-hcc
Copy link
Member

Choose a reason for hiding this comment

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

@powerkimhub 아 그렇군요 확인해주셔서 감사합니다! 참고하도록 하겠습니다

Please sign in to comment.