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

docker image -> buildpack, New Relic support, proxy support #28

Merged
merged 11 commits into from
Apr 28, 2022

Conversation

mogul
Copy link
Contributor

@mogul mogul commented Jan 25, 2022

Addresses GSA/data.gov#3062

Lots of refactoring happened to stop pushing a Docker image (a big compliance improvement)

  • We're only using the upstream Docker image for preparing plugins.
    • Other than that the Docker Compose situation resembles what we actually ship to cloud.gov as closely as possible.

Also adds:

  • Support for shipping logs to S3 through a proxy if AWS_S3_PROXY is set
  • Support for shipping logs to New Relic if the NEWRELIC_LICENSE_KEY env var is set
    • It's unclear if this output plugin respects https_proxy or not
    • There might be a way to set the proxy at the Java level so that it's not up to the plugins to decide whether they're going to support use it; worth investigating.

@mogul mogul requested a review from a team January 25, 2022 05:46
@mogul mogul force-pushed the docker-image-to-buildpack branch from dd72656 to 9162151 Compare January 25, 2022 21:46
mogul and others added 2 commits January 25, 2022 22:50
The tests pass locally, but the tests on github actions timed out three months ago, there's no logs and it doesn't seem like it's possible to re-run them
@nickumia-reisys
Copy link
Contributor

Figured out the problem with github actions,
image

On my hunt for a solution now.

Lots of testing went into this.. the summary, '.profile' needed files created from 'make logstash-installation' which could not complete without having write access to the 'logstash' folder
Copy link
Contributor

@nickumia-reisys nickumia-reisys left a comment

Choose a reason for hiding this comment

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

📖 Not sure if the NR stuff actually works. Are there any additional tests that we want to add?

@@ -0,0 +1,4 @@
logstash-installation:
curl "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o logstash/awscliv2.zip
curl https://artifacts.elastic.co/downloads/logstash/logstash-oss-7.16.3-linux-x86_64.tar.gz -o logstash/logstash-oss-7.16.3-linux-x86_64.tar.gz
Copy link
Contributor

Choose a reason for hiding this comment

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

The logstash version is hard-coded in a few places, do we want to consolidate that variable somewhere?

Copy link
Contributor

@nickumia-reisys nickumia-reisys left a comment

Choose a reason for hiding this comment

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

🚢

@nickumia-reisys nickumia-reisys merged commit 502746c into main Apr 28, 2022
@nickumia-reisys nickumia-reisys deleted the docker-image-to-buildpack branch April 28, 2022 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants