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

fix(update-server): Fix issues with 3.2 api on 3.3 system #2097

Merged
merged 1 commit into from
Aug 22, 2018

Conversation

sfoster1
Copy link
Member

Overview

This fixes issues with updating to a 3.2 api on a 3.3 system, and also issues
with running 3.2 on a 3.3 system.

API server wheels from before 3.3.0 do not have resource subdirectories and should not be
provisioned. The update server now inspects the version (from the filename) of the api wheel it is
installing and will not provision wheels whose version is prior to 3.3.0.

In addition, once the 3.2 api is uploaded, it will not have established the
proper setup scripts. This commit changes the container to properly fall back on
the scripts established in /usr/local/lib.

Also, the /restart changes that we thought we made were in the server lib which is not in fact used at all. They need to be in the update server's /restart.

review requests

Tested upgrade from 3.0.0 docker file and downgrade to 3.2 api (see comment below)

This fixes issues with updating to a 3.2 api on a 3.3 system, and also issues
with running 3.2 on a 3.3 system.

API server wheels from before 3.3.0 do not have resource subdirectories and should not be
provisioned. The update server now inspects the version (from the filename) of the api wheel it is
installing and will not provision wheels whose version is prior to 3.3.0.

In addition, once the 3.2 api is uploaded, it will not have established the
proper setup scripts. This commit changes the container to properly fall back on
the scripts established in /usr/local/lib.
@sfoster1 sfoster1 requested review from btmorr and mcous August 21, 2018 22:40
@sfoster1
Copy link
Member Author

Testing that's already accomplished

  1. Push 3.0 dockerfile to robot
  2. remove /data/system
  3. Update 3.3 api to robot: runs
{
    "name": "opentrons-moon-moon-moon",
    "api_version": "3.3.0-beta.1",
    "fw_version": "edge-6168d32",
    "logs": [
        "/logs/serial.log",
        "/logs/api.log"
    ]
}
  1. Push 3.3 dockerfile to robot and check that the correct path fallbacks exist
bash-4.3# bash -l
[ bash ] Configuring environment
[ bash ] Environment configuration done
[ bash ] Environment already configured
d9035d3:/# echo $PATH
/data/packages/usr/local/bin:/data/system/scripts:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/local/lib/python3.6/site-packages/opentrons/resources/scripts
d9035d3:/# which setup.sh
/data/system/scripts/setup.sh
d9035d3:/# mv /data/system/scripts /data/system/scripts2
d9035d3:/# which setup.sh
/usr/local/lib/python3.6/site-packages/opentrons/resources/scripts/setup.sh
d9035d3:/#
  1. Update 3.2 api to robot
{
    "message": [
        "Processing /opentrons-3.2.0-py2.py3-none-any.whl\nInstalling collected packages: opentrons\n  Found existing installation: opentrons 3.3.0b1\n    Uninstalling opentrons-3.3.0b1:\n      Successfully uninstalled opentrons-3.3.0b1\nSuccessfully installed opentrons-3.2.0",
        "Processing /ot2serverlib-3.2.0-py2.py3-none-any.whl\nInstalling collected packages: ot2serverlib\n  Found existing installation: ot2serverlib 3.3.0b1\n    Uninstalling ot2serverlib-3.3.0b1:\n      Successfully uninstalled ot2serverlib-3.3.0b1\nSuccessfully installed ot2serverlib-3.2.0"
    ],
    "filename": [
        "opentrons-3.2.0-py2.py3-none-any.whl",
        "ot2serverlib-3.2.0-py2.py3-none-any.whl"
    ]
}
  1. reboot with /restart

And afterwards the api server actually runs.

{
    "name": "opentrons-moon-moon-moon",
    "api_version": "3.2.0",
    "fw_version": "edge-6168d32"
}

@codecov
Copy link

codecov bot commented Aug 21, 2018

Codecov Report

Merging #2097 into edge will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge    #2097   +/-   ##
=======================================
  Coverage   32.86%   32.86%           
=======================================
  Files         453      453           
  Lines        7312     7312           
=======================================
  Hits         2403     2403           
  Misses       4909     4909

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe18e0e...a086fd5. Read the comment docs.

Copy link
Contributor

@btmorr btmorr left a comment

Choose a reason for hiding this comment

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

🐪 Code looks good. Given the testing detailed above, and the fact that we're doing release/compatibility testing starting tomorrow, I'm ok with merging this in now and including test of this feature as part of that.

@sfoster1 sfoster1 merged commit bad6e3a into edge Aug 22, 2018
@sfoster1 sfoster1 deleted the update_fix-old-api-installs branch August 22, 2018 12:27
b-cooper pushed a commit that referenced this pull request Aug 29, 2018
This fixes issues with updating to a 3.2 api on a 3.3 system, and also issues
with running 3.2 on a 3.3 system.

API server wheels from before 3.3.0 do not have resource subdirectories and should not be
provisioned. The update server now inspects the version (from the filename) of the api wheel it is
installing and will not provision wheels whose version is prior to 3.3.0.

In addition, once the 3.2 api is uploaded, it will not have established the
proper setup scripts. This commit changes the container to properly fall back on
the scripts established in /usr/local/lib.
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.

2 participants