Remove logging for copying of TLS certificates

[hferentschik]

We do the following logging as part of the service-manager env docker call - https://github.com/projectatomic/vagrant-service-manager/blob/master/lib/vagrant-service-manager/command.rb#L182:

@env.ui.info("# Copying TLS certificates to #{secrets_path}")

My understanding was that the copying of the certificates actually happens during provisioning of the VM (some time during vagrant up, probably just when the certificates are getting generated). At this stage the logging makes sense and sit useful. As part of the env docker call it is confusing and should be removed.

BTW, moving the certificate generation into startup, the experience of calling env docker has already improved a lot. Much snappier. :-)

[bexelbie]

The copy is actually done when you see that message:

https://github.com/projectatomic/vagrant-service-manager/blob/master/lib/vagrant-service-manager/command.rb#L182

Subsequent runs of the plugin should not give you that message.

We do this because we do not know if hte user will need the certs until they ask for the env. A vagrant ssh user, for example, does not need the certs locally.

[hferentschik]

Subsequent runs of the plugin should not give you that message.

It doesn't, but that's part of the confusion as well. Right now it is not really obvious that this is really an info message. It gets mixed in to the actual env docker output.

We do this because we do not know if hte user will need the certs until they ask for the env. A vagrant ssh user, for example, does not need the certs locally.

Sure, but it does not hurt to have the certs in place, right?

[bexelbie]

@hferentschik Perhaps you could open a separate issue to change the default behavior to be always copy the certs out.

For this issue can you suggest a format that would make it clearer in your opinion?

[coolbrg]

+1 for need of suggestion.

[hferentschik]

See also #121

[hferentschik]

For this issue can you suggest a format that would make it clearer in your opinion?

Just disable/remove it. It was intended as an info message. I was actually the one asking for it, but my understanding was that the copying will occur at the time the certificates get generated. There are actually good reasons for doing so as well - #21 (comment)

[hferentschik]

See issue #122

[navidshaikh]

Here is the current state of logging for TLS certs

• Display message in the logs when user does vagrant up ->

  Copying TLS certificates to /secrets/path/docker

• Display message as part of the command vagrant service-manager env docker only if the certs are copied (when there is a mismatch)

  # Copying TLS certificates to /secrets/path/docker

Are there any more modifications we want discuss / propose or are we good to close the issue? ping @hferentschik

[LalatenduMohanty]

I will suggest to take this out from ADB 2.0 milestone. At this point of time we should only fix blocker bugs for 2.0.

[coolbrg]

@hferentschik Ping

[hferentschik]

We are talking about removing some logging. This to be trivial and very low risk to me.

[bexelbie]

@hferentschik based on what @navidshaikh wrote in #120 (comment)

I think this in good shape. The logging message only shows up when there is a change made to the user's environment. If we want to silently update the environment, I believe we should try to have a wider convo on the ML.

[hferentschik]

If we want to silently update the environment, I believe we should try to have a wider convo on the ML.

Sure whatever. I am just surprised how a "feature" I initially asked for got a life in its own. Without me, this log message would not even exist. I asked for it in the first place. Mind you though, always only as part of a 'vagrant up'. I have been saying from the start that I don't think mixing this type of log information into the 'service-manager env' call makes sense. AFAIK there is no formal requirement for similar for that. So I am indeed surprised that I lost control over my own issue ;-)

[navidshaikh]

@hferentschik : Is there any action we want to perform as part of closing this issue? else please close this.

[navidshaikh]

Closing the issue. If we need additional logging or we need to remove some logging please create a new issue.