Skip to content

Conversation

@jaer-tsun
Copy link
Contributor

What this PR does / why we need it:
Adding check to prevent AzureNetworkContainer.exe from running if nc exists and the version has not changed.

@saiyan86 saiyan86 changed the title Adding check to prevent AzureNetworkContainer.exe from running if nc … Adding check to prevent duplicate Network Container creation Jul 6, 2018
break

// create/update nc only if it doesn't exist or it exists and the requested version is different from the saved version
if !ok || (ok && existing.VMVersion != req.Version) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be better to exit case early. Example:

if (ok && existing.VMVersion) = req.Version {
    break
}

if err = nc.Create(req); err != nil {
.
.
.
}

case "POST":
if req.NetworkContainerType == cns.WebApps {
// try to get the saved nc state if it exists
service.lock.Lock()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this lock necessary?

@jaer-tsun jaer-tsun force-pushed the check_nc_update_version branch from bfa2f06 to b51cdfa Compare July 6, 2018 22:50
@jaer-tsun jaer-tsun force-pushed the check_nc_update_version branch from b51cdfa to 66c7831 Compare July 6, 2018 22:52
Copy link
Member

@tamilmani1989 tamilmani1989 left a comment

Choose a reason for hiding this comment

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

lgtm

@tamilmani1989 tamilmani1989 merged commit 948a206 into Azure:master Jul 6, 2018
@jaer-tsun jaer-tsun deleted the check_nc_update_version branch July 16, 2018 22:41
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.

3 participants