Add Support for storing logs to Google Cloud Storage #552

khrm · 2023-08-07T17:12:13Z

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you review them:

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Tested your changes locally (if this is a code change)
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user-facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contain the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Add Support for storing logs to Google Cloud Storage. Set LogType to "GCS",
pass `GOOGLE_APPLICATION_CREDENTIALS` environment variable  and attach 
the file in the location pointed by `GOOGLE_APPLICATION_CREDENTIALS` variable.

khrm

/kind feature

khrm · 2023-08-07T17:16:31Z

/assign @enarha @sayan-biswas

tekton-robot · 2023-08-10T12:59:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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:

~~OWNERS~~ [vdemeester]

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

adambkaplan

Generally looks good - my comments are mainly nits on function names and log messages. It took me a while to understand what a lot of the test code was doing.

As a follow up, we should update https://github.com/tektoncd/results/blob/main/docs/DEVELOPMENT.md to include guidance on how to emulate the different log storage providers. Alternatively, we could add a separate doc that focuses on this topic.

EDIT: filed #559 for the follow up.

adambkaplan · 2023-08-10T20:37:36Z

pkg/api/server/config/config.go

@@ -29,6 +29,9 @@ type Config struct {
 	LOGS_BUFFER_SIZE int    `mapstructure:"LOGS_BUFFER_SIZE"`
 	LOGS_PATH        string `mapstructure:"LOGS_PATH"`

+	GCS_BUCKET_NAME       string `mapstructure:"GCS_BUCKET_NAME"`
+	STORAGE_EMULATOR_HOST string `mapstructure:"STORAGE_EMULATOR_HOST"`


Is this intended to be GCS specific? If so, please prefix this config item with GCS:

Suggested change

STORAGE_EMULATOR_HOST string `mapstructure:"STORAGE_EMULATOR_HOST"`

GCS_STORAGE_EMULATOR_HOST string `mapstructure:"GCS_STORAGE_EMULATOR_HOST"`

GCS driver uses this exact value. It uses the STORAGE_EMULATOR_HOST environment variable.

Please add a comment above with this information.

pkg/api/server/v1alpha2/log/gcs.go

pkg/api/server/v1alpha2/log/gcs_test.go

adambkaplan · 2023-08-10T20:55:43Z

pkg/api/server/v1alpha2/log/gcs.go

+	if cfg.STORAGE_EMULATOR_HOST != "" {
+		creds, _ = google.CredentialsFromJSON(ctx, []byte(`{"type": "service_account", "project_id": "my-project-id"}`))
+	} else {


Is there a project that emulates GCS storage (unauthenticated)? Is this something useful for contributors who want to test this without launching on GCP/GKE?

Yes. There is one storage emulator in python. A couple of storage in go.
This one works fine: https://github.com/fullstorydev/emulators

Logs can be stored to GCS. GOOGLE_APPLICATION_CREDENTIALS environment variable needs to be passed to api-server alongwith bucketname.

enarha

Apart from the test issue, the rest is LGTM.

enarha · 2023-08-13T08:29:07Z

pkg/api/server/v1alpha2/log/gcs_test.go

+)
+
+// This function is to create a replay or record http client depending
+// upon whether a -record flag was passed durin testing


typo: during

pkg/api/server/v1alpha2/log/gcs_test.go

enarha · 2023-08-14T09:53:01Z

/lgtm

tekton-robot added the release-note-none Denotes a PR that doesnt merit a release note. label Aug 7, 2023

tekton-robot requested review from alan-ghelardi and dibyom August 7, 2023 17:12

tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesnt merit a release note. labels Aug 7, 2023

khrm commented Aug 7, 2023

View reviewed changes

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 7, 2023

tekton-robot assigned enarha and sayan-biswas Aug 7, 2023

khrm force-pushed the f/gcs branch 3 times, most recently from 76f86d0 to fd39398 Compare August 8, 2023 08:03

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2023

khrm force-pushed the f/gcs branch 2 times, most recently from 19048db to 6884a65 Compare August 10, 2023 10:56

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 10, 2023

Vendor gocloud.dev, gcs driver and httpreplay to add support for GCS

5f10b7b

khrm force-pushed the f/gcs branch 3 times, most recently from 7bb8862 to 1d53de3 Compare August 10, 2023 12:17

vdemeester approved these changes Aug 10, 2023

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 10, 2023

adambkaplan reviewed Aug 10, 2023

View reviewed changes

khrm force-pushed the f/gcs branch 2 times, most recently from 77656fc to be8a16e Compare August 11, 2023 10:01

Add Support for storing logs to GCS

ffb62b4

Logs can be stored to GCS. GOOGLE_APPLICATION_CREDENTIALS environment variable needs to be passed to api-server alongwith bucketname.

khrm force-pushed the f/gcs branch from be8a16e to ffb62b4 Compare August 11, 2023 10:03

adambkaplan mentioned this pull request Aug 11, 2023

Add Developer docs for logs feature #559

Open

enarha reviewed Aug 13, 2023

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 14, 2023

tekton-robot merged commit 876bbf2 into tektoncd:main Aug 14, 2023
5 checks passed

khrm deleted the f/gcs branch September 6, 2023 11:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Support for storing logs to Google Cloud Storage #552

Add Support for storing logs to Google Cloud Storage #552

khrm commented Aug 7, 2023 •

edited

khrm left a comment

khrm commented Aug 7, 2023

tekton-robot commented Aug 10, 2023

adambkaplan left a comment •

edited

adambkaplan Aug 10, 2023

khrm Aug 11, 2023

adambkaplan Aug 11, 2023

adambkaplan Aug 10, 2023

khrm Aug 11, 2023

enarha left a comment

enarha Aug 13, 2023

enarha commented Aug 14, 2023

	STORAGE_EMULATOR_HOST string `mapstructure:"STORAGE_EMULATOR_HOST"`
	GCS_STORAGE_EMULATOR_HOST string `mapstructure:"GCS_STORAGE_EMULATOR_HOST"`

Add Support for storing logs to Google Cloud Storage #552

Add Support for storing logs to Google Cloud Storage #552

Conversation

khrm commented Aug 7, 2023 • edited

Changes

Submitter Checklist

Release Notes

khrm left a comment

Choose a reason for hiding this comment

khrm commented Aug 7, 2023

tekton-robot commented Aug 10, 2023

adambkaplan left a comment • edited

Choose a reason for hiding this comment

adambkaplan Aug 10, 2023

Choose a reason for hiding this comment

khrm Aug 11, 2023

Choose a reason for hiding this comment

adambkaplan Aug 11, 2023

Choose a reason for hiding this comment

adambkaplan Aug 10, 2023

Choose a reason for hiding this comment

khrm Aug 11, 2023

Choose a reason for hiding this comment

enarha left a comment

Choose a reason for hiding this comment

enarha Aug 13, 2023

Choose a reason for hiding this comment

enarha commented Aug 14, 2023

khrm commented Aug 7, 2023 •

edited

adambkaplan left a comment •

edited