Avoid allocate conflict IP address when cVM is powered off from vSphere #8410
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,7 +152,19 @@ func handleEvent(netctx *Context, ie events.Event) { | |
| if err := handle.Commit(op, nil, nil); err != nil { | ||
| op.Warnf("Failed to commit handle after network unbind for container %s: %s", ie.Reference(), err) | ||
| } | ||
| case events.ContainerStarted: | ||
| op := trace.NewOperation(context.Background(), fmt.Sprintf("handleEvent(%s)", ie.EventID())) | ||
| op.Infof("Handling Event: %s", ie.EventID()) | ||
| // grab the operation from the event | ||
| handle := exec.GetContainer(op, uid.Parse(ie.Reference())) | ||
| if _, err := netctx.BindContainer(op, handle); err != nil { | ||
| op.Warnf("Failed to bind container %s: %s", ie.Reference(), err) | ||
| return | ||
| } | ||
|
|
||
| if err := handle.Commit(op, nil, nil); err != nil { | ||
|
There was a problem hiding this comment. This will not have the effect you expect - exec.Commit will not make guestinfo changes while the VM is powered on. This is due to a vsphere bug that converts persistent guestinfo keys to volatile keys if modified while powered on. This bug has been fixed but I believe the unpatched ESX versions are still in LTS. We would need to check the behavior or patch version of each ESX host in the cluster (and any added to the cluster) to change this behavior. We had been looking to use namespacedb instead of guestinfo to bypass this issue. See 4062 and related. There was a problem hiding this comment. @hickeng based on my test with this patch, we have already reserved a new IP address and configured to the cVM when VM is powered on via vSphere client. But without this patch, I tried it on 1.4.3 and 1.5, the VM will use old IP address while it is powered on, I guess it is because the IP address is saved in guestinfo. There was a problem hiding this comment. You may be benefiting from a timing delay in your testing. vic/lib/portlayer/exec/handle.go Line 229 in 44d6f81 You may need to introduce a delay into the vsphere event loop processing to hit this case as I don’t know of a way to induce delays on the vsphere side. There was a problem hiding this comment. Hi George, you are right. I found for some cases, the guestinfo is changed and IP address is updated for container VM; and for some cases (I tried the image httpd), the guestinfo is not changed and the container VM is still using the old IP address which may result in IP conflict. But if we do not fix this issue, we may see issues #8405 #8052 and #7128. Would you mind we add a powerOff VM ops when we see the vSphere event from out of band and then reserve an IP address to update guestInfo? Is there any disadvantage? Another problem is why not adding a warning popup from vSphere client to avoid the operation from here? For example, in VIO any VM created from OpenStack will be tagged and it will throw a warning popup if customer wants to operate it from vSphere. |
||
| op.Warnf("Failed to commit handle after network bind for container %s: %s", ie.Reference(), err) | ||
| } | ||
| } | ||
| return | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
defer handle.Close() here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should also check that the handle isn’t nil - it likely won’t be, however the way the handle cache is written means you’re not assured there will be an entry.