ITISFoundation / osparc-simcore

🐼 osparc-simcore simulation framework
https://osparc.io
MIT License
46 stars 27 forks source link

Question: Why is `integration-version` in simcore dynamice services' docker container labels not respected? #3729

Closed mrnicegyu11 closed 1 year ago

mrnicegyu11 commented 1 year ago

@GitHK reports that the way director-v2 checks if a service is a dynamic sidecar service (and not legacy) is by inspecting the container image labels' label simcore.service.paths-mapping:

docker inspect registry.osparc-master.speag.com/simcore/services/dynamic/jupyter-math:2.0.8 | jq ".[0].Config.Labels" | grep "simcore.service.paths-mapping"

However, the container image label io.simcore.integration-version also exists and is sometimes (not always) used. The director-v2 however (according to @GitHK ) ignores this labels when checking if a service is legacy/dynamic.

Question to @mguidon @sanderegg @pcrespov : @GitHK reports that there was a reason why io.simcore.integration-version is not used by the director-v2, does anyone have memory or insights here?

In any case, my proposal is that the director-v2 also asserts that integration-version=2.0.0 and fails with an error if it is not set. I personally don't see much use in a version number if we don't use it and do duck-typing on the label simcore.service.paths-mapping instead. Although the priority here is low, this seems to me to be a good housekeeping tasks. Or removing the integration-version label from all services, this also would re-establish consistency.

My idea would be to first have services with inconsistent docker labels fail (on master/staging) and then fix the services. Thoughts?

mrnicegyu11 commented 1 year ago

For posterity, this is the script I run to check the integration version of simcore services in a remote library

#
# You need to run `docker login` on your target registries before running this script
#

import requests, warnings, os
import subprocess
warnings.filterwarnings(
    "ignore",
    ".*Adding certificate verification is strongly advised.*",
)
session = requests.Session()
################################
### CONFIG
################################
REG_DOMAIN="registry.osparc.local"
session.auth = (
    "admin", #USERNAME
    "testertester", #PASSWORD
}
MUSTMATCH_STRING='simcore/services/dynamic'
################################
hed = {
    "Content-Type": "application/json",
    "Accept": "application/json",
    "X-Requested-By": "cli",
}
registryList = session.get('https://' + REG_DOMAIN + "/v2/_catalog",headers=hed, verify=False)
for i in [a for a in registryList.json()['repositories'] if MUSTMATCH_STRING in a]:
    x = session.get('https://' + REG_DOMAIN + "/v2/" + i + "/tags/list",headers=hed, verify=False)
    foundAtLeastOneNotLegacy=False
    foundAtLeastOneLegacy=False
    for j in x.json()['tags']:
        cmdString= 'docker container run -i --rm --net host -v regctl-conf:/home/appuser/.regctl/ -u "$(id -u):$(id -g)" -e HOME -v $HOME:$HOME -v /etc/docker/certs.d:/etc/docker/certs.d:ro regclient/regctl:latest image config ' + REG_DOMAIN + '/' + i + ':' + j + ' | jq ".config.Labels.\\"io.simcore.integration-version\\""'
        proc = subprocess.Popen(cmdString, stdout=subprocess.PIPE, shell=True)
        out = proc.communicate()[0].decode("utf-8")
        if "1.0.0" in out or "null" in out and not "2.0.0" in out:
            foundAtLeastOneLegacy=True  
            print("Legacy:", str(REG_DOMAIN + '/' + i + ':' + j))
        else:
            foundAtLeastOneNotLegacy=j
    if foundAtLeastOneLegacy and foundAtLeastOneNotLegacy:
        print("FOUND NON LEGACY:", str(REG_DOMAIN + '/' + i + ':' + foundAtLeastOneNotLegacy))
    elif not foundAtLeastOneNotLegacy:
        print("WARNING -- ALL LEGACY:", str(REG_DOMAIN + '/' + i))
sanderegg commented 1 year ago

@mrnicegyu11 So for posterity, the service integration version was originally designed for computational services. These also have different versions which allow different things to happen. When a computational service runs, it expects some specifics as to what is mounted, and how. This is defined by the integration-version label.

I am not sure why this made it into the dynamic services and if it has any meaning there. I do agree this generates confusion. I guess that one problem stems from the fact that every oSparc service follows the same docker label requirements. But removing the label is definitely not an option since it will break the computational backend. I could imagine we could rename it maybe, but I am not sure that will improve your script ;)

mrnicegyu11 commented 1 year ago

ok thanks for that info.

sanderegg commented 1 year ago

since documentation was added by @mrnicegyu11 about how to find legacy services. I think we can close this.