ceph-volume: fix stderr failure to decode/encode when redirected

[alfredodeza]

This has taken a tremendous amount of work, and it is rolling back most/all of the automatic encoding/decoding effort done in 77912c0c718

In addition to the removal, I am making sure that

• previous tests are passing almost without modifying
• new functional tests demonstrate the problem and ensure the fix (need hooks into the CI though)
• adds a fallback when all fails with the logging module
• adds tests that verify that the logging module is used when encoding/decoding is failing

This PR should not be definite, and there should be new work put into place to fully remove the custom terminal logging so that the logging module is used instead. Since it is critical to have this fixed right away, my intention is to move this forward all the way to luminous and then start thinking about a proper solution to the terminal logging problem.

Fixes: https://tracker.ceph.com/issues/41660

[jan--f]

Code-wise this looks good to me. I'm wondering though, this patch falls back to using the terminal logger if _writer.write throws something. Why not use the terminal logger right away?

This PR should not be definite, and there should be new work put into place to fully remove the custom terminal logging so that the logging module is used instead. Since it is critical to have this fixed right away, my intention is to move this forward all the way to luminous and then start thinking about a proper solution to the terminal logging problem.

Agreed. Having terminal.write use the terminal logger should get us there most of the way though, shouldn't it? Not pretty, but the unnecessary code can be stripped out at a later stage.

[votdev]

Suggested change

	# we can't set sys.stderr as the default for _writer. otherwise
	# we can't set sys.stderr as the default for _writer, otherwise

[alfredodeza]

Code-wise this looks good to me. I'm wondering though, this patch falls back to using the terminal logger if _writer.write throws something. Why not use the terminal logger right away?

There are more things involved if we want to use the logger. Internally, ceph-volume makes a distinction about stderr and stdout from commandline tools, those have color outputs, and no indication of log levels etc... It would take a bit more effort to get there, I think.

Also, when the work started, I thought it was going to be easier/simpler but it ended up requiring some sort of fallback which happened towards the end.

My preference would be to focus in the fix (with the fallback) and try to get a better solution with logging in a separate PR. I am happy to address any potential problems you see here though. Let's try to make sure this works really well

[jan--f]

Code-wise this looks good to me. I'm wondering though, this patch falls back to using the terminal logger if _writer.write throws something. Why not use the terminal logger right away?

There are more things involved if we want to use the logger. Internally, ceph-volume makes a distinction about stderr and stdout from commandline tools, those have color outputs, and no indication of log levels etc... It would take a bit more effort to get there, I think.

Alright, should have guessed that things are more complicated then that ;)

My preference would be to focus in the fix (with the fallback) and try to get a better solution with logging in a separate PR. I am happy to address any potential problems you see here though. Let's try to make sure this works really well

Sounds good to me. Feel free to get in touch to discuss solutions.

[alfredodeza]

@andrewschoen @tchaikov if you could also take a look please... this is a big fix and want to make sure I am getting it 100% right

[alfredodeza]

jenkins test ceph-volume tox

[andrewschoen]

I think this looks good. I'm thinking we should hook in the new functional testing into the CI eventually. If this is really urgent maybe we should not block on that though.