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

Avoid allocate conflict IP address when cVM is powered off from vSphere #8410

Closed
wants to merge 1 commit into from

Conversation

yuyangbj
Copy link
Contributor

When container VM is powered off from vSphere client, the VIC will
receive the PoweredOff event and invoke the unbindContainer method
to release IP address. But VIC does not do any update if the cVM
is powered on from vSphere client, so cVM will use the previous
netlink configuration, VIC will allocate the released IP address to
the next container VM, this will result in IP conflict.

Fixes #8405

…rom vSphere

When container VM is powered off from vSphere client, the VIC will
receive the PoweredOff event and invoke the unbindContainer method
to release IP address. But VIC does not do any update if the cVM
is powered on from vSphere client, so cVM will use the previous
netlink configuration, VIC will allocate the released IP address to
the next container VM, this will result in IP conflict.
@yuyangbj yuyangbj requested a review from a team as a code owner December 13, 2018 09:35
@yuyangbj yuyangbj requested review from hickeng and zjs December 13, 2018 09:42
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()))
Copy link
Contributor

Choose a reason for hiding this comment

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

defer handle.Close() here?

Copy link
Member

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.


if err := handle.Commit(op, nil, nil); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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.

Copy link
Member

@hickeng hickeng Dec 17, 2018

Choose a reason for hiding this comment

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

You may be benefiting from a timing delay in your testing.
IP address is a persistent configuration element (to allow for HA mediated restart of cVMs) and will be stripped out of the config if the cVM is powered on.

filter = extraconfig.NonPersistent | extraconfig.Hidden

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@yuyangbj
Copy link
Contributor Author

Using PR #8445 for tracking, so closing it.

@yuyangbj yuyangbj closed this Feb 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants