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

CI-999: Modify console template on creation to add tlog #243

Merged
merged 1 commit into from
Dec 10, 2021

Conversation

VSpike
Copy link
Contributor

@VSpike VSpike commented Nov 24, 2021

This PR addresses CI-999 (adding tlog into the console job template on creation) and CI-1010 (adding a sidecar to the console to read the session data and stream to Pubsub).

It modifies the console operator to do the following:

  • Add a volume mount for the log files
  • Add the volume mount to the main container(s)
  • Modify the main container(s) commands to wrap everything in tlog
  • Add a sidecar with the appropriate image
  • Pass through configuration data as env vars to the sidecar
  • Create a role and rolebinding that will allow the console pod to read information about itself

@VSpike VSpike self-assigned this Nov 24, 2021
@VSpike VSpike added the WIP label Nov 24, 2021
@VSpike VSpike force-pushed the CI-999/modify_console_template branch from 574e165 to edce413 Compare November 24, 2021 15:55
@VSpike VSpike force-pushed the CI-999/modify_console_template branch 3 times, most recently from e53f80f to 217eb54 Compare December 6, 2021 19:12
@VSpike VSpike force-pushed the CI-999/modify_console_template branch from 217eb54 to 4e221d2 Compare December 9, 2021 16:44
@VSpike VSpike removed the WIP label Dec 9, 2021
@VSpike VSpike requested a review from a team December 9, 2021 16:47
Copy link
Contributor

@benwh benwh left a comment

Choose a reason for hiding this comment

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

Looks good overall - some minor comments

controllers/workloads/console/controller.go Outdated Show resolved Hide resolved
controllers/workloads/console/controller.go Show resolved Hide resolved
controllers/workloads/console/controller.go Show resolved Hide resolved
controllers/workloads/console/controller.go Show resolved Hide resolved
corev1.ResourceCPU: resource.MustParse("512m"),
},
Requests: corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("512Mi"),
Copy link
Contributor

Choose a reason for hiding this comment

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

These seem pretty beefy, but happy to go with it and tune it down later on if we see that it's excessive

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 definitely generous but we're not sure what the actual requirements are yet.

controllers/workloads/console/controller.go Outdated Show resolved Hide resolved
controllers/workloads/console/controller.go Outdated Show resolved Hide resolved
@VSpike VSpike requested a review from benwh December 9, 2021 18:24
benwh
benwh previously approved these changes Dec 10, 2021
Copy link
Contributor

@benwh benwh left a comment

Choose a reason for hiding this comment

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

LGTM

benwh
benwh previously approved these changes Dec 10, 2021
CI-1010: Modify console operator to add sidecar

* Create a volume to write the tlog data to
* Add the volume mount to the main container
* Modify the main container commands to wrap everything in tlog
* Add a sidecar that runs our wrapper and pubsubtle (log data pusher)
* Add rolebinding for service account to read pod status
* Handle case of missing service account name
* Add topic ID and project ID as flags and pass through to sidecar
@VSpike VSpike force-pushed the CI-999/modify_console_template branch from 110a2dc to 9cad315 Compare December 10, 2021 12:36
Copy link
Contributor

@benwh benwh left a comment

Choose a reason for hiding this comment

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

LGTM after squash

@VSpike VSpike merged commit 13e679d into master Dec 10, 2021
@VSpike VSpike deleted the CI-999/modify_console_template branch December 10, 2021 14:46
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