Current behavior
Background
The following commits aim to remove support for the V1 Loggregator API:
However, by doing so, these changes now also require the actual initialization of the newV2IngressClient() in the diego-logging-client code.
Where as before, if the loggregator.* properties were missing/undefined on the vxlan-policy-agent job, the diego-logging-client would simply initialize a noopIngressClient.
In our case, the latter was the case. We did not have any of the loggregator.* properties defined on the vxlan-policy-agent job, which had worked fine up until now. Or at least, it didn't cause any jobs to fail.
However, during our last deployment now, and without any errors from the templating engine in BOSH about missing required properties, the deployment failed to start the Diego Cell instances with the following error:
cfnetworking: initializing ingress client: failed to load keypair: tls: failed to find any PEM data in certificate input
This was especially painful to discover during deployment (and not templating), since this is one of our larger Cloud Foundry deployments, which takes a few hours to deploy...
Root Cause
Since the above mentioned commits now force the full initialization of the newV2IngressClient(), which in turn requires all of the TLS properties, the following templating using if_p() is misleading/incorrect:
<% if_p("loggregator.ca_cert") do |value| %><% if_p("loggregator.cert") do |value| %><% if_p("loggregator.key") do |value| %>
Because these are now effectively required properties, without any default values in the spec file.
Desired behavior
Describe how the problem should be fixed
Change the loggregator.* properties to be required by switching from if_p() to p() in the above mentioned templates.
This will ensure to surface the required properties to the deployment/manifest operator, notifyng them that this release now requires TLS certificates for the Loggregator client, should those properties not be defined on the job definition in the deployment.
Does this require a new bosh release?
No.
Does it require configuration changes in cf-deployment?
No.
Do we need to have a special release note?
No. People who are already affected by this bug, have probably fixed it already. This is a pure quality-of-life change, to surface these requirements earlier during the templating phase, instead of failing during deployment phase.
Do we need to update repo documentation?
No. The Loggregator Client integration does not seem to be explicitly mentioned in any of the docs/* files. The spec file for the vxlan-policy-agent already indicates that these are required properties, by not providing any default: values (as is natural for certificate properties).
Affected Version
3.111.0
Current behavior
Background
The following commits aim to remove support for the V1 Loggregator API:
However, by doing so, these changes now also require the actual initialization of the
newV2IngressClient()in thediego-logging-clientcode.Where as before, if the
loggregator.*properties were missing/undefined on thevxlan-policy-agentjob, thediego-logging-clientwould simply initialize anoopIngressClient.In our case, the latter was the case. We did not have any of the
loggregator.*properties defined on thevxlan-policy-agentjob, which had worked fine up until now. Or at least, it didn't cause any jobs to fail.However, during our last deployment now, and without any errors from the templating engine in BOSH about missing required properties, the deployment failed to start the Diego Cell instances with the following error:
This was especially painful to discover during deployment (and not templating), since this is one of our larger Cloud Foundry deployments, which takes a few hours to deploy...
Root Cause
Since the above mentioned commits now force the full initialization of the
newV2IngressClient(), which in turn requires all of the TLS properties, the following templating usingif_p()is misleading/incorrect:<% if_p("loggregator.ca_cert") do |value| %><% if_p("loggregator.cert") do |value| %><% if_p("loggregator.key") do |value| %>Because these are now effectively required properties, without any default values in the
specfile.Desired behavior
Describe how the problem should be fixed
Change the
loggregator.*properties to be required by switching fromif_p()top()in the above mentioned templates.This will ensure to surface the required properties to the deployment/manifest operator, notifyng them that this release now requires TLS certificates for the Loggregator client, should those properties not be defined on the job definition in the deployment.
Does this require a new bosh release?
No.
Does it require configuration changes in cf-deployment?
No.
Do we need to have a special release note?
No. People who are already affected by this bug, have probably fixed it already. This is a pure quality-of-life change, to surface these requirements earlier during the templating phase, instead of failing during deployment phase.
Do we need to update repo documentation?
No. The Loggregator Client integration does not seem to be explicitly mentioned in any of the
docs/*files. Thespecfile for thevxlan-policy-agentalready indicates that these are required properties, by not providing anydefault:values (as is natural for certificate properties).Affected Version
3.111.0