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

Retrieve telemetry send status from server component #7591

Merged
merged 4 commits into from
Jan 25, 2022

Conversation

mrsimonemms
Copy link
Contributor

@mrsimonemms mrsimonemms commented Jan 13, 2022

Description

Creates the installation admin controller in the server component and associated database. This database works on the basis that there is only one record created per installation - in future, this will be the backing for the installation's admin panel. At the moment, this only has one record (sendTelemetry), but will have more records in future.

The sendTelemetry boolean currently defaults to false and will do until the admin panel exists then it will change to default true.

It amends the Installer to cater for these changes. It allows the gitpod-telemetry job to access the server component. It also removed the GITPOD_DOMAIN_HASH and uses the randomly generated ID for installation identification.

Related Issue(s)

Fixes #7571
Fixes #7585

How to test

Deploy via werft and check that you can access GET:/installation-admin/data when port forwarding to the server component.

Release Notes

[server]: Create installation admin controller

Documentation

@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #7591 (4b52267) into main (e335309) will not change coverage.
The diff coverage is n/a.

❗ Current head 4b52267 differs from pull request most recent head 8033623. Consider uploading reports for the commit 8033623 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7591   +/-   ##
=======================================
  Coverage   10.38%   10.38%           
=======================================
  Files          18       18           
  Lines         992      992           
=======================================
  Hits          103      103           
  Misses        888      888           
  Partials        1        1           
Flag Coverage Δ
components-gitpod-cli-app 10.38% <ø> (ø)
components-installation-telemetry-app ∅ <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e5d13a...8033623. Read the comment docs.

@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch from b082f71 to e516a9d Compare January 13, 2022 15:47
@roboquat roboquat added size/M and removed size/S labels Jan 13, 2022
@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch from e516a9d to 57b9d46 Compare January 13, 2022 16:44
@roboquat roboquat added team: delivery Issue belongs to the self-hosted team size/L and removed size/M labels Jan 13, 2022
@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch 2 times, most recently from 3190c7d to 7719042 Compare January 13, 2022 17:10
@roboquat roboquat added size/XL and removed size/L labels Jan 13, 2022
@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch from 7719042 to e5f8ae2 Compare January 14, 2022 09:23
@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Jan 14, 2022

/werft run

👍 started the job as gitpod-build-sje-server-installation-admin.6

@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Jan 14, 2022

/werft run

👍 started the job as gitpod-build-sje-server-installation-admin.7

@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Jan 14, 2022

/werft run

👍 started the job as gitpod-build-sje-server-installation-admin.8

@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch 2 times, most recently from fcd02b4 to ce66d27 Compare January 17, 2022 13:05
@mrsimonemms
Copy link
Contributor Author

mrsimonemms commented Jan 17, 2022

/werft run

👍 started the job as gitpod-build-sje-server-installation-admin.11

@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch 2 times, most recently from e942e8f to 45d3525 Compare January 17, 2022 14:59
@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch from 45d3525 to 192bedf Compare January 17, 2022 15:35
@mrsimonemms mrsimonemms marked this pull request as ready for review January 17, 2022 15:36
@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch 4 times, most recently from 0017185 to bcfbe27 Compare January 21, 2022 08:59
@corneliusludmann
Copy link
Contributor

Looks good from my side. Would like to leave the final /lgtm to Gero because of the server/DB changes.

/approve

@corneliusludmann
Copy link
Contributor

@mrsimonemms This is ready for review, right? Could we remove the hold label?

@mrsimonemms
Copy link
Contributor Author

/unhold

@corneliusludmann yes - left on my mistake

Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

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

LGTM!

(sorry for the delay 🙏 )

@roboquat roboquat added the lgtm label Jan 25, 2022
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: a207956f456e8c67fb7e087db670e5fd87a53c72

@mrsimonemms mrsimonemms force-pushed the sje/server-installation-admin branch from bcfbe27 to 8033623 Compare January 25, 2022 09:20
@roboquat roboquat removed the lgtm label Jan 25, 2022
@mrsimonemms mrsimonemms added this to the release/2022.01 milestone Jan 25, 2022
@mrsimonemms
Copy link
Contributor Author

/lgtm
/approve

@roboquat
Copy link
Contributor

@mrsimonemms: you cannot LGTM your own PR.

In response to this:

/lgtm
/approve

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@geropl
Copy link
Member

geropl commented Jan 25, 2022

/lgtm

@roboquat roboquat added the lgtm label Jan 25, 2022
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 7889540a246a1bfaaf1ba8695e6f4a7ca576c83a

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: corneliusludmann, geropl, MrSimonEmms

Associated issue: #7571

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit 4f2e71e into main Jan 25, 2022
@roboquat roboquat deleted the sje/server-installation-admin branch January 25, 2022 11:26
@Pothulapati Pothulapati mentioned this pull request Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved release-note size/XXL team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team
Projects
None yet
4 participants