Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is Jaeger restart required after change to cert generation script? #1011

Closed
pavolloffay opened this issue Apr 9, 2020 · 7 comments
Closed
Labels
Elasticsearch The issues related to Elasticsearch storage

Comments

@pavolloffay
Copy link
Member

pavolloffay commented Apr 9, 2020

Follow up from #1008 (comment)

A simple test showed that certs (elasticsearch and simple-prod-jaeger-elasticsearch) are not changed if Jaeger operand is created before the change to the script.

I have done these steps to validate it:

  1. Deploy Jaeger operator before PR Synch changes in cert generation script with CLO #1008
  2. Create Jaeger operand oc create -f deploy/examples/openshift/simple-prod-deploy-es.yaml
  3. Deploy Jaeger operator with PR Synch changes in cert generation script with CLO #1008

Elasticsearch or Jaeger pods haven't been redeployed and the secrets haven't changed. I am attaching diff diff /tmp/_certs/project1/simple-prod/cf03e514-59ac-4d4f-855b-869a0c284d45 original_certs - new cet directory vs old one before #1008 .

Diff in cert directory

diff /tmp/_certs/project1/simple-prod/3957a21b-15b4-4953-b5a2-c7fe89e23298 original_certs

diff.txt

Cmd to get secrets

oc get secrets elasticsearch -o yaml > ~/tmp/elasticsearch.yaml               
oc get secrets simple-prod-jaeger-elasticsearch -o yaml > ~/tmp/simple-prod-jaeger-elasticsearch.yaml

cc) @ewolinetz

@ghost ghost added the needs-triage New issues, in need of classification label Apr 9, 2020
@pavolloffay pavolloffay added Elasticsearch The issues related to Elasticsearch storage and removed needs-triage New issues, in need of classification labels Apr 9, 2020
@pavolloffay
Copy link
Member Author

I have also tested one additional scenario that involves changing Elasticsearch CR to trigger the reconciliation loop. I have changed the number of nodes from 1 to 3 after step 3. The secrets haven't changed either.

@ewolinetz
Copy link

For cluster logging, CLO is responsible for updating the secrets -- which will only be different if the contents of the certs has changed. The script itself doesn't change the certs unless the signing CA has expired or if the signing CA had to be regenerated.

@pavolloffay
Copy link
Member Author

Thanks @ewolinetz. So, in this case, we are good and we don't have to do anything for existing deployments.

For cluster logging, CLO is responsible for updating the secrets -- which will only be different if the contents of the certs has changed.

How is this use-case handled in CLO? As we are using the same script the certs do not change.

In a separate issue, we should start thinking how to restart Jaeger components when certs expire or change.

@ewolinetz
Copy link

How is this use-case handled in CLO? As we are using the same script the certs do not change.

We run the script during every reconcile loop, we pull our certs down to a local dir in the operator pod, so the cert gen script can also detect if they are expired.

Then we create or update our secrets every time, so if the local certs have changed then the secrets will get updated as well if necessary.

@pavolloffay
Copy link
Member Author

We run the script during every reconcile loop, we pull our certs down to a local dir in the operator pod, so the cert gen script can also detect if they are expired.

Then we create or update our secrets every time, so if the local certs have changed then the secrets will get updated as well if necessary.

Jaeger operator does the same. If the certs are not present on operators filesystem then the certs are extracted from secrets. Then the script is run to validate the certs (e.g. whether they are expired)

I have run the diff on the certs directory before PR #1008 and after - see the first comment. There are some changed files but the core certs haven't changed e.g. elasticsearch.cert only some "help" files .conf .csr.

diff.txt

@ewolinetz
Copy link

If the certs aren't expired this is what we would expect to see.. or are you saying since the default_md and default_bits changed we should be regenerating the certs?

@pavolloffay
Copy link
Member Author

I think it is correct how it is handled right now. From your first comments my impressions were that new set of certs will be generated if PR #1008 is applied to existing deployments.

@ewolinetz thanks for the confirmation. I am closing this and I will open a new issue for handling Jaeger restart when certs change - e.g. expire.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Elasticsearch The issues related to Elasticsearch storage
Projects
None yet
Development

No branches or pull requests

2 participants