Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

[logstash] Add fullnameOverride setting #457

Merged
merged 2 commits into from
Jan 31, 2020

Conversation

morganchristiansson
Copy link
Contributor

@morganchristiansson morganchristiansson commented Jan 22, 2020

Added fullnameOverride to logstash by copy pasting from kibana/filebeat/metricbeat charts.

  • Chart version not bumped (the versions are all bumped and released at the same time)
  • README.md updated with any new values or changes
  • Updated template tests in ${CHART}/tests/*.py
  • Updated integration tests in ${CHART}/examples/*/test/goss.yaml

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@morganchristiansson
Copy link
Contributor Author

morganchristiansson commented Jan 23, 2020

This is for overriding the name of the logstash service and just generally better naming options than "logstash-logstash" and "logstash-main-main" allowed by setting nameOverride value today.

With fullnameOverride the logstash service object can be given any name

Please run the build in Jenkins as I did not run tests locally.

@jmlrt
Copy link
Member

jmlrt commented Jan 24, 2020

jenkins test this please

@dorkvodor
Copy link

Hiya! Can this be merged now?

@morganchristiansson
Copy link
Contributor Author

morganchristiansson commented Jan 28, 2020

I managed to run the tests locally and created PR #463 to fix incompatible attrs dependency error for make pytest.

I updated tests to remove checks that are not applicable for default logstash deployment and it's passing locally now.

Please re-run the jenkins tests

@jmlrt
Copy link
Member

jmlrt commented Jan 31, 2020

jenkins test this please

Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

} in volumes
assert custom_name in r['statefulset']
assert r['statefulset'][custom_name]['spec']['template']['spec']['containers'][0]['name'] == 'logstash'
volumes = r['statefulset'][custom_name]['spec']['template']['spec']['volumes']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last line volumes = ... doesn't seem to be used

Copy link
Contributor Author

@morganchristiansson morganchristiansson Jan 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where I copied it from it was used for this check

    assert {
               'name': 'data',
               'hostPath': {
                   'path': '/var/lib/' + custom_name + '-default-data',
                   'type': 'DirectoryOrCreate'
               }
           } in volumes

I don't think we care about custom_name being in volume mount path... So I'm going to delete this.

I do care that logstash.fullname has correct value and is used as the name of the deployment, service, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rebased my branch on upstream/master, amended this change into the Update tests commit and force-pushed

Added fullnameOverride to logstash by copy pasting from kibana/filebeat/metricbeat charts
Copy link
Member

@jmlrt jmlrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmlrt
Copy link
Member

jmlrt commented Jan 31, 2020

jenkins test this please

@jmlrt jmlrt merged commit 8e10ead into elastic:master Jan 31, 2020
@morganchristiansson
Copy link
Contributor Author

Thanks! I assume this will be released in the next elastic-stack release? Current release is 7.5.2 so 7.5.3 and later will include this fix?

@jmlrt
Copy link
Member

jmlrt commented Feb 1, 2020

Yes, this will be released in the next one which should be 7.5.3 or 7.6.0.

@morganchristiansson morganchristiansson deleted the logstash-fullnameoverride branch May 6, 2021 18:45
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants