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

[INF-93] Remove rsyslog implementation and build Logspout sidecar container #3124

Merged
merged 59 commits into from
Jun 1, 2022

Conversation

joaquincasares
Copy link
Contributor

@joaquincasares joaquincasares commented May 19, 2022

Description

Build a custom Logspout container that contains:

  • A healthcheck
  • Default options
  • Baked in CMD to minimize docker-compose.yml configuration.

Sister PR that uses this image can be found here:

Tests

How will this change be monitored? Are there sufficient logs?

discovery-provider/scripts/start.sh Show resolved Hide resolved
discovery-provider/scripts/start.sh Outdated Show resolved Hide resolved
discovery-provider/compose/docker-compose.common.yml Outdated Show resolved Hide resolved
discovery-provider/scripts/start.sh Show resolved Hide resolved
@joaquincasares joaquincasares changed the title Build a custom Logspout container for SPs Remove rsyslog implementation and build Logspout sidecar container May 19, 2022
@AudiusProject AudiusProject deleted a comment from gitguardian bot May 19, 2022
@gitguardian
Copy link

gitguardian bot commented May 19, 2022

⚠️ GitGuardian has uncovered 5 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
3509188 Generic High Entropy Secret 809d593 .circleci/config.yml View secret
3509188 Generic High Entropy Secret 8ed1fe4 .circleci/config.yml View secret
3509188 Generic High Entropy Secret 8ed1fe4 .circleci/config.yml View secret
3509188 Generic High Entropy Secret 0692511 .circleci/config.yml View secret
3509188 Generic High Entropy Secret 0692511 .circleci/config.yml View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@joaquincasares joaquincasares changed the title Remove rsyslog implementation and build Logspout sidecar container [INF-93] Remove rsyslog implementation and build Logspout sidecar container May 25, 2022
Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

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

Overall this looks great, just a few questions and small comments

.circleci/config.yml Show resolved Hide resolved
creator-node/scripts/start.sh Outdated Show resolved Hide resolved
creator-node/scripts/start.sh Outdated Show resolved Hide resolved
logspout/start.sh Outdated Show resolved Hide resolved
discovery-provider/scripts/start.sh Show resolved Hide resolved
@joaquincasares joaquincasares requested a review from dmanjunath May 26, 2022 23:17
Copy link
Contributor

@dmanjunath dmanjunath left a comment

Choose a reason for hiding this comment

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

this is looking very good, just had a few clarifying questions and one suggested change with identity

@@ -34,11 +34,13 @@ ARG git_sha
ARG audius_loggly_disable
ARG audius_loggly_token
ARG audius_loggly_tags
ARG audius_enable_rsyslog
Copy link
Contributor

Choose a reason for hiding this comment

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

i know i've gone back and forth on this, but could we remove these changes from identity. since this is not run by third parties this should be easy to change later if necessary. we don't need the default enable like we do for content and discovery nodes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing! Created this draft PR to house these changes: #3182

@@ -0,0 +1,16 @@
# Build Custom Audius Logstash Sidecar Container
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand what this readme is trying to explain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, that was meant to say Logspout, not Logstash.

All files within the logspout directory are related to the Dockerfile and this README allows us to build the image manually, if CircleCI wasn't working.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh got it. can we add a sentence or two to mention this explicitly? Like "In the event you want to build the Logspout container by hand (for resting purposes, CircleCI is down etc) run the following:"

@@ -0,0 +1,13 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

where is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are part of FROM gliderlabs/logspout:v3.2.14's ONBUILD command:

https://github.com/gliderlabs/logspout/blob/818dd8260e52d2c148280d86170bdf5267b5c637/Dockerfile#L9-L11

Copy link
Contributor

Choose a reason for hiding this comment

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

we should add this link in a comment on this file 🙂

tag_csv=${tag_csv},${audius_loggly_tags}
fi

hostname=${audius_discprov_url}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just do an if else for if either of these hostname env vars are defined? This reads a little confusing

hostname=${creatorNodeEndpoint}
fi
if [[ "${hostname}" ]]; then
hostname=$(echo ${hostname} | sed -e 's/[^/]*\/\/\([^@]*@\)\?\([^:/]*\).*/\2/')
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add comments to the whole file but especially here, no idea what's supposed to happen. If anyone could figure out right off the bat what this regex did I'd be very impressed

@joaquincasares joaquincasares merged commit 1cd4c73 into master Jun 1, 2022
@joaquincasares joaquincasares deleted the jc-inf-93 branch June 1, 2022 20:22
dmanjunath pushed a commit that referenced this pull request Jun 2, 2022
…tainer (#3124)

* Build a custom Logspout container for SPs

* rm rsyslog

* stop tee'ing to logger

* separate one process per container

* missing audius_service envar

* mild fixes before revert

* Revert "breaking up dp container"

This reverts commit 74ad707.

* Revert "missing audius_service envar"

This reverts commit ecd0ea3.

* Revert "separate one process per container"

This reverts commit 2609fa0.

* rm last syslog calls

* use audius_loggly_token everywhere

* background job

* image tagging

* update build command

* Dockerfile update

* th

* kansas city shuffle

* fix building

* remove healthcheck

* fix Dockerfile ordering and loggly tagging

* fix circleci param

* fix shuffle

* set tags correctly

* sh not bash

* Add more log tags

* default logspout-tag

* quotes

* no logspout test

* true string

* dir

* mv logging/logspout logspout

* repo: logspout

* mv logging/logspout logspout

* remove redundant tagging

* add hostname, if it exists

* Update default_config.ini

* Update helpers.py

* gate rsyslog

* restore discovery-provider/scripts/rotate-log.sh

* chmod 755

* gate for dp

* gate rsyslog

* keep rsyslog integration with circleci

* fix newlines

* ordering

* restore dockerfiles

* add manual approval req for logstash image building

* use true/false instead of 0/1 for envars

* default local to DEBUG logs

* extract hostname from cn and dp nodes

* audius_discprov_loglevel_flask

* formatting

* Update README.md

* Update start.sh

* revert changes for identity service

* add additional comment

* address PR comment

* add README context
sliptype pushed a commit that referenced this pull request Sep 10, 2023
[a82b6d2] [C-2391 PLAT-833] Handle get-discovery-notification error, notification processing (#3141) Dylan Jeffers
[b6a8470] [PAY-1073] Mobile messages settings screen (#3131) Reed
[c567b35] Separate chat text input to own component (#3142) Reed
[1a38cb1] [PAY-1090] - Subscribe to special access gates to unlock new tracks (#3108) Saliou Diallo
[9e5a45d] Set dev optimizely key (#3126) Sebastian Klingler
[c5e86fa] [PLAT-699] Add tastemaker notification to web (#3140) Dylan Jeffers
[4bc608c] [C-2295] Only show now-playing-artwork-tile when song enqueued (#3137) Dylan Jeffers
[aa34b32] [PAY-1039] Mobile chat header navs to profile (#3139) Reed
[55f5819] Optimizely attributes - use null if empty instead of undefined (#3134) nicoback2
[92ab79e] [C-2387] Improve permalink and user handle caching (#3117) Dylan Jeffers
[086af81] [PAY-1107][PAY-1112] Center mobile chat text input, enlarge icon (#3136) Reed
[fcec3f5] [PAY-1123] Fix mobile chat scroll being stuck (#3135) Reed
[a276e86] Revert "Allow Optimizely targeting w CodePush on mobile C-2394" (#3133) nicoback2
[3335b4a] [C-2357] Update the icon color of the podcast skip buttons to accept the theme color (#3132) Kyle Shanks
[1af5730] Fix dapp store versionCode (#3121) Dylan Jeffers
[7787115] Fix app store version fetched from Apple in Fastlane being out of date first 24h after new release C-2397 (#3128) nicoback2
[06c181d] Allow Optimizely targeting w CodePush on mobile C-2394 (#3127) nicoback2
[ff07c32] [PAY-1038] Fix mobile profile page button borders (#3130) Reed
[9286959] [PAY-1027] Mobile chat websockets (#3129) Reed
[81ef3b3] [PAY-1031] Mobile chat list views use full height (#3123) Reed
[41e3f5d] [C-2231] Remove audius-data, dynamically import web3modal (#1771) Raymond Jacobson
[34d80be] [PAY-1111] Remove "Done" row on ios keyboard for mobile chats (#3124) Reed
[9eb83a8] [PAY-1102] - Add supporters info drawer (#3112) Saliou Diallo
[6fe7abe] Add dependencies, bundle to mobile README (#3111) Reed
[dc8d4f2] Show header shadow in chat screens (#3106) Reed
[f871a38] [PAY-906] Mobile chats KeyboardAvoidingView (#3122) Reed
[fd00bf2] Fix chat reactions with createEntityAdapter (#3118) Reed
[9876137] Switch to 1s (#3120) Raymond Jacobson
[fe9efea] Tip using both erc_wallet and wallet (#3049) Michael Piazza
[fdfc0b6] Update dapp store readme (#3119) Raymond Jacobson
[ca58e69] [C-2359, C-2380] Update duration text for podcast tiles (#3116) Kyle Shanks
[6441421] [PLAT-695] Add trending notifications to valid_types (#3109) Dylan Jeffers
[4233ee6] [PAY-1083] - Only allow prompt modal when single track is being uploaded (#3115) Saliou Diallo
[36909d1] Always include user signature for stream (#3114) Raymond Jacobson
[b90eaf6] [C-2386] Fix notification button not toggling panel (#3110) Dylan Jeffers
[a3c2e91] Check if messagesStatus undefined in ChatScreen (#3113) Reed
[64d89ca] [C-2382] Update dapp-store cli, short_description length (#3104) Dylan Jeffers
[680931d] [C-2361, C-2360, C-2355, C-2346] Update skip 15s button behavior on mobile for podcasts (#3105) Kyle Shanks
[3eb742f] [C-2353, C-2358] Update playback position tracking for web and mobile (#3097) Kyle Shanks
[e06c4d9] Bump sdk to 2.0.3-beta.1 (#3098) Isaac Solo
[8191694] Fix "Pick some for me" button on android (#3102) Sebastian Klingler
@AudiusProject AudiusProject deleted a comment from linear bot Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants