TITUS-2326 Cloned titus-log-viewer executable. Preparing shared log … #162
Conversation
rickkw
commented
Jul 30, 2018
rickkw
commented
- Cloned titus-darion to titus-log-viewer executable
- Modified darion shared code to work with log root in both inside and outside of container
…iewer code to work in both inside and outside container.
| } | ||
|
|
||
| func newMux() *http.ServeMux { | ||
| r := http.NewServeMux() |
There was a problem hiding this comment.
If you're cloning this, it probably makes sense to split up this (shared) code.
| @@ -39,8 +39,8 @@ func LogHandler(w http.ResponseWriter, r *http.Request) { | |||
| } | |||
|
|
|||
| func logHandler(w http.ResponseWriter, r *http.Request, containerID, fileName string) { | |||
| filePath := filepath.Join(conf.ContainersHome, containerID, "logs", fileName) | |||
| fout, err := os.Open(filePath) | |||
| logPath := filepath.Join(buildLogLocationBase(containerID), fileName) | |||
There was a problem hiding this comment.
I don't know if the cloned version is meant to be multi-tenant, or single-tenant. If it's meant to be single-tenant (i.e. Darion per container), then you should remove everything around containerID.
There was a problem hiding this comment.
@sargun the goal of this PR is to fudge the darion code just enough that allows both multi-tenant and single-tenant to work (initially), so I can work on injecting it into the container. When I commented about cloning, I referred to the executable, titus-log-viewer. The shared darion stays mostly intact. This is not meant to be the final code.
I don't want to spend time refactoring this code (just yet). Rather, I think it's better to focus on the process injection which has more unknowns.
There was a problem hiding this comment.
Then I would keep this on your own branch for now, and wait to merge until it's ready, because this part of the code isn't touched very much.
|
|
||
| // TitusTaskID initialized using TITUS_TASK_ID from OS environment variable. | ||
| // It can be used as a crude way to determine if this process is running inside a container. | ||
| TitusTaskID = os.Getenv("TITUS_TASK_ID") |
There was a problem hiding this comment.
See Above, RE: multi-tenant vs. single-tenant.
Pull Request Test Coverage Report for Build 1795
💛 - Coveralls |
Codecov Report
@@ Coverage Diff @@
## master #162 +/- ##
==========================================
+ Coverage 33.76% 33.78% +0.01%
==========================================
Files 63 64 +1
Lines 7543 7560 +17
==========================================
+ Hits 2547 2554 +7
- Misses 4689 4697 +8
- Partials 307 309 +2
|
| logsutil.MaybeSetupLoggerIfUnderSystemd() | ||
| log.Println("Titus log viewer is starting") | ||
| r := newMux() | ||
| if err := http.ListenAndServe(":8004", r); err != nil { |
There was a problem hiding this comment.
Is this meant to run in a different network namespace where it won't clash on port with Darion?
| "testing" | ||
| ) | ||
|
|
||
| func TestPing(t *testing.T) { |
There was a problem hiding this comment.
What is this test actually testing? Looks like it's just using golang http packages.
There was a problem hiding this comment.
honest I don't know. I just cloned the test from darion.
|
@rickkw are you still looking for reviews here, or should we wait until you have done testing in your own branch and are ready to open the final PR? |
|
@fabiokung per @sargun advice, I am leaving this PR open while working on injecting Darion inside containers. No need to merge this PR and introduce risk, while this effort is still WIP. I won't be merging this PR until I make progress with the actual injection work. |
sargun
deleted the
TITUS-2326_prepare_log_viewer_to_run_inside_container
branch
