Connector CI: Update nightlies to write to GCS #26929

bnchrch · 2023-06-01T23:58:59Z

What

We need our nightly builds to output test results to a different path

How

We added a new bucket airbyte-connector-integration-reports
We updated all connector commands to output to GCS
We updated the report path to be constructed from the CI context

Example output file from a local run
https://storage.googleapis.com/airbyte-connector-integration-reports/test/manual/bnchrch_dagger_update-nightlies/1685688402/f63bd2708013d1cec9d9051b4c9b4328b9bcafff/source-marketo/1.1.0/output.json

closes: #26397

CLAassistant · 2023-06-01T23:59:05Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Octavia Squidington III seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

bnchrch · 2023-06-02T00:01:52Z

tools/ci_connector_ops/ci_connector_ops/pipelines/contexts.py

@@ -357,6 +360,18 @@ def metadata(self) -> dict:
    def docker_image_from_metadata(self) -> str:
        return f"{self.metadata['dockerRepository']}:{self.metadata['dockerImageTag']}"

+    @property
+    def ci_gcs_credentials_secret(self) -> Secret:
+        # TODO (ben): Update this to be in use ANYWHERE we use a service account.


@alafanechere I didnt want to add yet another service account env var. It felt like we were going down a bad way there.

Im thinking that we should enforce one service account for the entire pipeline. WDYT?

@alafanechere Heres a draft of that change: #26931

Yes it'd be much better to have a SA for airbyte-ci connectors that can do the thing it has to do for metadata upload, report upload etc...

bnchrch · 2023-06-02T00:05:07Z

.github/workflows/connector_nightly_builds_dagger.yml

  workflow_dispatch:
    inputs:
      runs-on:
        type: string
        default: dev-large-runner
        required: true
      test-connectors-options:
-        default: --concurrency=5 --release-stage=generally_available --release-stage=beta
+        default: --concurrency=5 --release-stage=generally_available


@alafanechere I was noticing some flakyness even when only running GA connectors.

Im thinking we start the nightlies off with GA, then if they match the current nightlies we expand to beta

I was able to successfully run a couple of nightlies on both GA and beta so I think we should keep it like it is now.

alafanechere · 2023-06-02T17:09:42Z

.github/workflows/connector_nightly_builds_dagger.yml

  workflow_dispatch:
    inputs:
      runs-on:
        type: string
        default: dev-large-runner
        required: true
      test-connectors-options:
-        default: --concurrency=5 --release-stage=generally_available --release-stage=beta
+        default: --concurrency=5 --release-stage=generally_available


I was able to successfully run a couple of nightlies on both GA and beta so I think we should keep it like it is now.

alafanechere · 2023-06-02T17:17:13Z

.github/workflows/publish_connectors.yml

@@ -31,7 +28,7 @@ jobs:
      METADATA_SERVICE_BUCKET_NAME: prod-airbyte-cloud-connector-metadata-service
      SPEC_CACHE_BUCKET_NAME: io-airbyte-cloud-spec-cache
      SPEC_CACHE_GCS_CREDENTIALS: ${{ secrets.SPEC_CACHE_SERVICE_ACCOUNT_KEY_PUBLISH }}
-      TEST_REPORTS_BUCKET_NAME: "airbyte-connector-build-status"
+      TEST_REPORTS_BUCKET_NAME: "airbyte-connector-integration-reports"


We might reuse this bucket for non connector use case?

Suggested change

TEST_REPORTS_BUCKET_NAME: "airbyte-connector-integration-reports"

TEST_REPORTS_BUCKET_NAME: "airbyte-ci-reports"

alafanechere · 2023-06-02T17:20:29Z

tools/ci_connector_ops/ci_connector_ops/pipelines/commands/groups/connectors.py

+    cmd = ctx.invoked_subcommand
+
+    path_values = [
+        cmd,


alafanechere · 2023-06-02T17:22:24Z

.github/workflows/connector_integration_test_single_dagger.yml

-          AWS_SECRET_ACCESS_KEY: ${{ secrets.STATUS_API_AWS_SECRET_ACCESS_KEY }}
-          AWS_DEFAULT_REGION: "us-east-2"
-          TEST_REPORTS_BUCKET_NAME: "airbyte-connector-build-status"
+          TEST_REPORTS_BUCKET_NAME: "airbyte-connector-integration-reports"


Suggested change

TEST_REPORTS_BUCKET_NAME: "airbyte-connector-integration-reports"

TEST_REPORTS_BUCKET_NAME: "airbyte-ci-reports"

I believe we could use this bucket for other use cases. I'd suggest to use a common root path like connectors to group all connectors related reports;

alafanechere · 2023-06-02T17:24:06Z

tools/ci_connector_ops/ci_connector_ops/pipelines/commands/groups/connectors.py

@@ -55,11 +55,46 @@ def validate_environment(is_local: bool, use_remote_secrets: bool):
        )


+def render_report_output_prefix(ctx: click.Context) -> str:


I believe this function can be a method of the Report class, given you'd pass the invoked_command to our PipelineContext class.

I left this in as I think its a good idea for the airbyte-ci to set the top level path for all airbyte-ci related outputs.

This lets us standardize the path in both the pipelines and any utilities we might make.

alafanechere · 2023-06-02T17:58:21Z

tools/ci_connector_ops/ci_connector_ops/pipelines/contexts.py

@@ -357,6 +360,18 @@ def metadata(self) -> dict:
    def docker_image_from_metadata(self) -> str:
        return f"{self.metadata['dockerRepository']}:{self.metadata['dockerImageTag']}"

+    @property
+    def ci_gcs_credentials_secret(self) -> Secret:
+        # TODO (ben): Update this to be in use ANYWHERE we use a service account.


Yes it'd be much better to have a SA for airbyte-ci connectors that can do the thing it has to do for metadata upload, report upload etc...

alafanechere · 2023-06-02T17:59:38Z

tools/ci_connector_ops/ci_connector_ops/pipelines/contexts.py

+
+    @property
+    def test_report_bucket(self) -> str:
+        bucket = os.environ.get("TEST_REPORTS_BUCKET_NAME")  # TODO (ben): move to config


Shall we move it to the context instance? (I believe its what you mean by config)

alafanechere · 2023-06-02T18:04:47Z

tools/ci_connector_ops/ci_connector_ops/pipelines/contexts.py

-        git_branch = self.report.pipeline_context.git_branch.replace("/", "_")
-        suffix = f"{connector_name}/{git_branch}/{connector_version}/{git_revision}.json"
-        local_report_path = Path(local_reports_path_root + suffix)
+        connector_name = self.connector.technical_name


Just to get the proper reference I made sure to use the context attached to the report to retrieve these metadata.

alafanechere · 2023-06-02T18:06:04Z

tools/ci_connector_ops/ci_connector_ops/pipelines/contexts.py

+        connector_name = self.connector.technical_name
+        connector_version = self.connector.version
+
+        suffix = f"{connector_name}/{connector_version}/output.json"
+        file_path_key = f"{self.report_output_prefix}/{suffix}"
+
+        local_report_path = Path(local_reports_path_root + file_path_key)
+


nit: shall we create a dedicated method that would generate GCS and local path?

Im thinking here that we mandate that the relative path stays the same remote vs local

Then the things are configurable are

What bucket to write to if remote

What root folder to write to if tmp

Keeps the path to report consistent between both

alafanechere · 2023-06-02T18:10:11Z

tools/ci_connector_ops/ci_connector_ops/pipelines/contexts.py

        if self.report.should_be_saved:
-            s3_key = self.s3_report_key + suffix
-            report_upload_exit_code = await remote_storage.upload_to_s3(


For a soft roll out could we keep uploading reports to both S3 and GCS?

@alafanechere as I understand this is due to not wanting to break the dashboard correct?

I think no matter what those get broken with the new paths.

So perhaps we just go over to gcs

I'd like to keep the "old" path for S3 to not break the dashboard.

bnchrch requested a review from alafanechere June 1, 2023 23:59

bnchrch commented Jun 2, 2023

View reviewed changes

Octavia Squidington III added 7 commits June 2, 2023 14:29

Generate report output prefix

11b9bbd

DNC

983dab4

Upload to gcs bucket

d453de4

Revert dev comments

61bbacc

Format

31af476

Reenable for cron

aa90f16

remove reports

b8f7b55

bnchrch force-pushed the bnchrch/dagger/update-nightlies branch from 46e4598 to b8f7b55 Compare June 2, 2023 21:33

alafanechere reviewed Jun 2, 2023

View reviewed changes

Octavia Squidington III added 5 commits June 5, 2023 14:56

Merge origin/master

2819e35

Connector CI reports

7c51451

Reremove AWS

f2af62d

Missed some env vars

c6dfa9f

Move bucket to CI

2c762c3

alafanechere approved these changes Jun 6, 2023

View reviewed changes

Octavia Squidington III added 3 commits June 6, 2023 00:36

Merge origin/master

8569e83

Bad merge

12ba356

Merge origin/master

b2cc13c

bnchrch enabled auto-merge (squash) June 6, 2023 16:30

bnchrch merged commit 2df72cc into master Jun 6, 2023
17 of 18 checks passed

bnchrch deleted the bnchrch/dagger/update-nightlies branch June 6, 2023 16:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connector CI: Update nightlies to write to GCS #26929

Connector CI: Update nightlies to write to GCS #26929

bnchrch commented Jun 1, 2023 •

edited

CLAassistant commented Jun 1, 2023

bnchrch Jun 2, 2023

bnchrch Jun 2, 2023

alafanechere Jun 2, 2023

bnchrch Jun 2, 2023

alafanechere Jun 2, 2023

alafanechere Jun 2, 2023

alafanechere Jun 2, 2023

alafanechere Jun 2, 2023

alafanechere Jun 2, 2023

alafanechere Jun 2, 2023

bnchrch Jun 5, 2023

alafanechere Jun 2, 2023

alafanechere Jun 2, 2023

alafanechere Jun 2, 2023

alafanechere Jun 2, 2023

bnchrch Jun 5, 2023

alafanechere Jun 2, 2023

bnchrch Jun 5, 2023

bnchrch Jun 6, 2023

alafanechere Jun 6, 2023

	TEST_REPORTS_BUCKET_NAME: "airbyte-connector-integration-reports"
	TEST_REPORTS_BUCKET_NAME: "airbyte-ci-reports"

		@@ -55,11 +55,46 @@ def validate_environment(is_local: bool, use_remote_secrets: bool):
		)


		def render_report_output_prefix(ctx: click.Context) -> str:

Connector CI: Update nightlies to write to GCS #26929

Connector CI: Update nightlies to write to GCS #26929

Conversation

bnchrch commented Jun 1, 2023 • edited

What

How

CLAassistant commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnchrch commented Jun 1, 2023 •

edited