see also https://tracker.ceph.com/issues/45631

mypy:

==================================== ERRORS ====================================
____________________ ERROR collecting tests/test_cephadm.py ____________________
tests/test_cephadm.py:11: in <module>
    cd = SourceFileLoader('cephadm', 'cephadm').load_module()
cephadm:2220: in <module>
    @container_registry_credentials()
cephadm:1160: in __init__
    if not args.registry_username:
E   NameError: name 'args' is not defined

thanks for the PR! Just a few minor questions:

1. do we need mgr/cephadm module options as well? otherwise no one will add the arguments?
2. we we need that in the unit files as well? I mean, podman run might also pull befor running the container.
3. fixes https://tracker.ceph.com/issues/44886 as well?
4. do you know https://www.mankier.com/5/containers-auth.json ?

mypy:

==================================== ERRORS ====================================
____________________ ERROR collecting tests/test_cephadm.py ____________________
tests/test_cephadm.py:11: in <module>
    cd = SourceFileLoader('cephadm', 'cephadm').load_module()
cephadm:2220: in <module>
    @container_registry_credentials()
cephadm:1160: in __init__
    if not args.registry_username:
E   NameError: name 'args' is not defined

thanks for the PR! Just a few minor questions:

1. do we need mgr/cephadm module options as well? otherwise no one will add the arguments?

Yes, we need a way to pass these options down when cephadm is called by mgr/cephadm.

2. we we need that in the unit files as well? I mean, podman run might also pull befor running the container.

Agreed. But I like the cached registry better. if we cannot have it sooner, i will have to add this in qa/ as well.

3. fixes https://tracker.ceph.com/issues/44886 as well?

I was not sure if it was an acceptable fix when working on this last night. But seems you are interested in this. And it’s actually a feature! not yet, i want to talk to @djgalloway to see if we can setup a http proxy to cache the images. my intention is to address https://tracker.ceph.com/issues/45631 ASAP as our tests are failing.

4. do you know https://www.mankier.com/5/containers-auth.json ?

i know. that's why i want to logout to avoid security issues by calling "logout". do you have better idea how to keep this cached credentials around without hurting the performance? how about having a global instance of this decorator / context manager, and call "logout" in its destructor?

1. do you know https://www.mankier.com/5/containers-auth.json ?

i know. that's why i want to logout to avoid security issues by calling "logout". do you have better idea how to keep this cached credentials around without hurting the performance? how about having a global instance of this decorator / context manager, and call "logout" in its destructor?

after a second thought, i am not sure about ir anymore. as "logout" command nukes all the cached credentials. no matter if they are added by cephadm or not.

but i am keeping it anyway. if you think i should remove it, i will do.

meh, i removed it, as it's difficult to use a class to define a decorator, and to initialize this decorator requires a variable (args) not set yet by then.

Caching within cephadm itself won't help.

1. There is typically only one container created per invocation. (the exception is bootstrap, which creates two containers in a single invocation)
2. deploying a daemon doesn't pull anything: pulling the containers is executed by `podman ru in the context of the systemd unit.
3. There is no such thing as a 'global instance'. src/cephadm is just a CLI tool without any persistent state.

Also, how do you make cephadm shell work without adding the registry credentials all the time?

Which makes me think. Having a solution where podman run just works ootb would be my preferred way to get this done.

https://pad.ceph.com/p/cephadm-registry-credentials

still this PR makes sense, if it is done right. @tchaikov you want to continue working on this?

this PR was created to address the test failures. since it has been fixed by you, its priority is now lowered. i will come back once i have more bandwidth.