cluster up: add option to install logging components #11343
Conversation
|
@jim-minter @bparees ptal |
| loggingDeployerTemplate = "logging-deployer-template" | ||
| ) | ||
|
|
||
| func instantiateTemplate(client client.Interface, mapper configcmd.Mapper, templateNamespace, templateName, targetNamespace string, params map[string]string) error { |
There was a problem hiding this comment.
we already have one of these for the jenkins admission controller, can we not reuse it/move it to a helper utility location?
There was a problem hiding this comment.
@bparees so I looked into merging this code with the one from the admission controller, but there's very little that we can actually share. Basically we could share the call to the bulk create. However, even then, you have a custom After handler in that code that is not necessarily what I want in this case.
|
I think you need @jcantrill's sign off more than mine. |
csrwng
force-pushed
the
clusterup_logging
branch
from
d44a9fd to
5ecdba0
Compare
|
I had understood the executive direction of deploying logging to be an Ansible playbook -- do we really want to maintain this as well? |
|
/cc @smarterclayton |
|
@stevekuznetsov so yes, for now ... ideally the deployer pod would do an Ansible install |
csrwng
force-pushed
the
clusterup_logging
branch
from
5ecdba0 to
9ea3449
Compare
| @@ -38,6 +38,7 @@ const ( | |||
|
|
|||
| var ( | |||
| openShiftContainerBinds = []string{ | |||
| "/var/log:/var/log:rw", | |||
There was a problem hiding this comment.
Out of interest, why is this being added?
There was a problem hiding this comment.
For aggregated logging to work, fluentd relies on the node to write symlinks in /var/log/containers that serve as metadata to the actual container logs. Without mounting /var/log the containerized node would be writing symlinks to its container filesystem where fluentd can't read them.
| if c.ShouldInstallLogging && c.ShouldInitializeData() { | ||
| loggingInfo = fmt.Sprintf("The kibana logging UI is available at:\n"+ | ||
| " https://%s\n\n", openshift.LoggingHost(c.RoutingSuffix, c.ServerIP)) | ||
| } |
There was a problem hiding this comment.
(As much a note to self as anything): this info should probably go into oc cluster status as well - see whether this PR beats #11171 or not!
|
This is installing the single system setup for logging to enable someone to run with it on in a semi-realistic way. |
|
@stevekuznetsov This is the short term fix to 'oc cluster up' providing a logging stack for someone to experience logging. In future, once ansible is availabe and packaged nicely in an image, this would need to be reworked to consume that change. |
|
My only comment is regarding the template. I don't know how much it has changed over the lifetime of logging but unfortunate it will need to be refreshed here when it changes. May be less of an issue once we are able to rely on ansible. Otherwise LGTM |
|
[test] |
|
Evaluated for origin test up to 9ea3449 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10069/) (Base Commit: d942e98) |
|
[merge] |
|
#11094 |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/10069/) (Image: devenv-rhel7_5206) |
|
#9203 |
|
Evaluated for origin merge up to 9ea3449 |
Adds
--loggingflag to cluster up to install log aggregation components