Comments (6)
I found that I did not submit my final keyring2.py. (It was still in my VM.) It's there now with a force push.
To experiment with yapf3, I created two additional commits and left them separate for others to see and comment as well. The difference between the two is the number of columns.
The one thing I would like to note is that it's possible to disable formatting for some sections. In the particular examples that I did here, I think this is the right tactic when displaying podman command lines. Compressing all the options and arguments into as few lines as possible does not look good.
All that said, yapf3 fixed my indenting mistake and recreated some structures which is something I would have preferred. I think I would be more in favor of adopting yapf3 and dropping pylint. If yapf3 and pylint ever disagree, then it's just more work for the submitter to appease a tool.
from deepsea.
Here you can find my approach to the problem: aec0ce0
taken from branch https://github.com/SUSE/DeepSea/compare/next...return_struct_next?expand=1
I went with the existing podman modules and extended it with a class that acts as a DataContainer
typ thing.
See here:
aec0ce0#diff-12fe82452ced2beebb7266e548e7ec19R23-R100
That basically implements a glorified return structure that also allows dynamic assignment of certain fields based on the return we get.
For shell invocation I decided to go with subprocess.run
(the why is explained in the comments of the code)
This allows us to set timeouts, retrieve the returncode, stdout, stderr and raise Exceptions based on the behavior. See here aec0ce0#diff-12fe82452ced2beebb7266e548e7ec19R172-R192
A module return output looks like this for me now:
local:
----------
command:
/usr/bin/podman run --rm --net=host -e CONTAINER_IMAGE=registry.suse.de/devel/storage/6.0/images/ses/6/ceph/ceph -e NODE_NAME=admin -v /srv/salt/ceph/bootstrap:/srv/salt/ceph/bootstrap --entrypoint /usr/bin/ceph-authtool registry.suse.de/devel/storage/6.0/images/ses/6/ceph/ceph --create-keyring /srv/salt/ceph/bootstrap/keyring --gen-key -n mon. --cap mon allow *
comment:
The function create_initial_keyring of module salt.loaded.ext.module.podman returned with code 0
err:
func_name:
create_initial_keyring
human_result:
success
module_name:
salt.loaded.ext.module.podman
out:
creating /srv/salt/ceph/bootstrap/keyring
rc:
0
result:
True
One layer up the stack we have do_x
(aec0ce0#diff-83b09f45ee8aa92de255d294c79a56ebR523)
That calls the LocalClient with a module.function.
The return is check with a newly implemented evaluate_module_return
function aec0ce0#diff-83b09f45ee8aa92de255d294c79a56ebR495
( This is necessary as the return from LocalClient contains a nested datastructure, basically the return from multiple minions)
do_x
returns a tuple which represents the overall/summarized return of all the affected minions and the new returnstructure.
Ultimately the do_x
function is called from a runner. aec0ce0#diff-8fd6811c61959f53acd72040a1674dd0R6
The runner inteprets the return based on it's context, which I haven't found an elegant solution so far.
Looking forward to comments/suggestions/improvements!
from deepsea.
I extended the return structure a bit and added the bit for CalledProcessError
:
The new commit can be found here c5eb793
updated: 1f4e1c0
local:
----------
command:
/usr/bin/podman run --rm --net=host -e CONTAINER_IMAGE=registry.suse.de/devel/storage/6.0/images/ses/6/ceph/ceph -e NODE_NAME=admin -v /srv/salt/ceph/bootstrap:/srv/salt/ceph/bootstrap --entrypoint /usr/bin/ceph-authtool registry.s
use.de/devel/storage/6.0/images/ses/6/ceph/ceph --doesnt exist
comment:
The function create_initial_keyring_failure of module podman returned with code 1
err:
/usr/bin/ceph-authtool: unexpected 'exist'
func_name:
create_initial_keyring_failure
guide:
Try running: salt '<target_minion>' podman.create_initial_keyring_failure
human_result:
failure
module_name:
podman
out:
No stdout captured
output:
usage: ceph-authtool keyringfile [OPTIONS]...
where the options are:
-l, --list will list all keys and capabilities present in
the keyring
-p, --print-key will print an encoded key for the specified
entityname. This is suitable for the
'mount -o secret=..' argument
-C, --create-keyring will create a new keyring, overwriting any
existing keyringfile
-g, --gen-key will generate a new secret key for the
specified entityname
--gen-print-key will generate a new secret key without set it
to the keyringfile, prints the secret to stdout
--import-keyring FILE will import the content of a given keyring
into the keyringfile
-n NAME, --name NAME specify entityname to operate on
-a BASE64, --add-key BASE64 will add an encoded key to the keyring
--cap SUBSYSTEM CAPABILITY will set the capability for given subsystem
--caps CAPSFILE will set all of capabilities associated with a
given key, for all subsystems
--mode MODE will set the desired file mode to the keyring
e.g: '0644', defaults to '0600'
rc:
1
result:
False
timout:
0
for the positive output it looks like this:
local:
----------
command:
/usr/bin/podman run --rm --net=host -e CONTAINER_IMAGE=registry.suse.de/devel/storage/6.0/images/ses/6/ceph/ceph -e NODE_NAME=admin -v /srv/salt/ceph/bootstrap:/srv/salt/ceph/bootstrap --entrypoint /usr/bin/ceph-authtool registry.s
use.de/devel/storage/6.0/images/ses/6/ceph/ceph --create-keyring /srv/salt/ceph/bootstrap/keyring --gen-key -n mon. --cap mon allow *
comment:
The function create_initial_keyring of module podman returned with code 0
err:
No stderr captured
func_name:
create_initial_keyring
guide:
No guidance needed
human_result:
success
module_name:
podman
out:
creating /srv/salt/ceph/bootstrap/keyring
output:
No output captured
rc:
0
result:
True
timout:
0
from deepsea.
The func_name looks good. I am wondering if module_name and func_name should just be one key. So, the value would be podman.create_initial_keyring
. (At some point, I would like to revisit the names themselves.)
With respect to CalledProcessError
changing the capturing of output depending on whether a command succeeded or failed, I'm not a fan. I also believe we can do without the canned responses. The exception handling isn't buying anything that we don't already have. I would be in favor of dropping the guide, human_result and output. (When I first saw output with out and err, I was confused about what would show up where.) Any guide content can be in comment; otherwise, the comment doesn't really have too much use.
I think the command, out, err, rc, some form of module/func and result are good. I'm uncertain about the timeout.
from deepsea.
update the commit with some more improvements: 1f4e1c0
I also factored multiple functions into non-salt managed files for re-usability.
from deepsea.
Another implementation using Python imports within Salt f26dff8. The Podman class in this example only builds the command. The cmd.run_all
remains in the module.
from deepsea.
Related Issues (20)
- make install fails on archlinux HOT 1
- cephdisks.unused falsely returns mounted disks
- Typo - should be c_v_commands instead of d_v_commands
- rebuild runner needs to read error messages from osd.py(runner) HOT 2
- osd.remove fails to zap devices on ceph version 14.2.3-349 HOT 10
- functests.3nodes fails with IndexError HOT 4
- How to deploy /dev/sdb ,/dev/sdc only use /dev/nvme0n1 as db device ? HOT 5
- Can not remove cluster node HOT 2
- [SES5] remove.osd functest fails HOT 9
- No role for rbd-client (e.g. mapping images) HOT 1
- Stage.3 Hang in Disks.Deploy HOT 6
- IGW lrbd support
- Device discovery not working in KVM environment HOT 3
- SES5: "time_init: ntp" does not work as expected HOT 1
- SES5: osd.redeploy fails for filestore -> bluestore
- deepsea hangs in stage 3 @ ceph.mgr.auth on
- deepsea monitor not working with gitfs
- stage.0 Exception: 'getpwuid(): uid not found:
- Deepsea development status after cephadm release HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from deepsea.