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

Updates to pre-upgrade container are dropped #7534

Open
hickeng opened this issue Mar 16, 2018 · 5 comments
Open

Updates to pre-upgrade container are dropped #7534

hickeng opened this issue Mar 16, 2018 · 5 comments
Labels
impact/test/integration/enable The test is associated with a disabled integration test kind/defect Behavior that is inconsistent with what's intended priority/p2 source/customer Reported by a customer, directly or via an intermediary team/container

Comments

@hickeng
Copy link
Member

hickeng commented Mar 16, 2018

Build: https://ci-vic.vmware.com/vmware/vic/17605/8
Suite: 11-01-Upgrade

Failing output because it contains Exited (0)

CONTAINER ID        IMAGE                                                  COMMAND                  CREATED              STATUS                     PORTS                                                      NAMES
<snip>
d865d2abeaaa        busybox                                                "sh"                     3 minutes ago        Exited (0) 3 seconds ago                                                              vch-restart-test1

The test is attempting to assert that the container from pre-upgrade can be started. It can, however because of a configuration version difference the resetting of started field (and times, et al) is being dropped (see this code) and we therefore don't block waiting for the start to occur.
This was not detected by the test prior to 6144b31 because the call to start explicitly set the container state to Starting - the test should have been checking for Running, not checking for the abscence of Exited. However with that commit we now use the vSphere event to trigger the state update and the test therefore still sees the old state.

There is a second issue related to this migration behaviour (#7128) and discarding IP address assignments. The only correct fix for this is to finish implementing the migration logic - we have the portion to migrate old configurations forward for reading, but not the portion that migrates new configurations back for writing (and should return a hard error if a critical element cannot be migrated).

It should be comparatively simple to add the reverse path to the existing migration code, but I do not know without investigating whether it will entail refactoring. Estimate as a 5 due to unknowns.

Related: #7128 #5653

@hickeng hickeng added kind/defect Behavior that is inconsistent with what's intended priority/p2 team/container labels Mar 16, 2018
@mdubya66 mdubya66 added priority/p0 source/ci Found via a continuous integration failure and removed priority/p2 labels Mar 20, 2018
@mdubya66
Copy link
Contributor

being seen on almost every CI run. changing to P0

@sgairo sgairo added this to the Sprint 29 Container Ops milestone Mar 27, 2018
@sgairo
Copy link
Contributor

sgairo commented Mar 28, 2018

This has been commented out in the test.

@vburenin vburenin assigned vburenin and unassigned vburenin Apr 3, 2018
@mdubya66 mdubya66 added priority/p2 impact/test/integration/enable The test is associated with a disabled integration test and removed source/ci Found via a continuous integration failure priority/p0 labels Apr 5, 2018
@mdubya66
Copy link
Contributor

mdubya66 commented Apr 5, 2018

This is existing behavior for upgrade, it's a p2 as @hickeng made it when filing. Resetting.

There is an integration test on this and when fixed needs to be revisited tests/test-cases/Group11-Upgrade/11-01-Upgrade.robot

@sgairo sgairo removed this from the Sprint 29 Container Ops milestone Apr 5, 2018
@hickeng hickeng added the source/customer Reported by a customer, directly or via an intermediary label Jun 16, 2018
@hickeng
Copy link
Member Author

hickeng commented Jun 16, 2018

I'm adding this to 1.5 as it's now been hit by a customer. @dbarkelew may know a bugXXX number to link with.

@hickeng
Copy link
Member Author

hickeng commented Dec 11, 2018

@YanzhaoLi To recreate this:
a. add a new plugin to lib/migration/plugins to force a data version migration after the upgrade (this is just a change for recreation - you don't want to ship the plugin unless there is an actual data format change).
b. start the container at least once before the upgrade - this is necessary to get the started field set to true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/test/integration/enable The test is associated with a disabled integration test kind/defect Behavior that is inconsistent with what's intended priority/p2 source/customer Reported by a customer, directly or via an intermediary team/container
Projects
None yet
Development

No branches or pull requests

4 participants