test: Add report creation to correctness test

[tristanvuong2021]

test: Add report creation to correctness test

[wfa-reviewable]

This change is 

[SanjayVas]

Reviewed 11 of 11 files at r1, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @tristanvuong2021)

---

.github/workflows/run-k8s-tests.yml line 55 at r1 (raw file):

        MC_API_KEY: ${{ secrets.MC_API_KEY }}
        GCLOUD_PROJECT: ${{ vars.GCLOUD_PROJECT }}
        REPORTING_PUBLIC_API_TARGET: v2alpha.reporting.dev.halo-cmm.org:8443

I went ahead and added the variable for all 3 environments

Suggestion:

${{ vars.REPORTING_PUBLIC_API_TARGET }}

---

src/test/kotlin/org/wfanet/measurement/integration/k8s/SyntheticGeneratorCorrectnessTest.kt line 51 at r1 (raw file):

 * Test for correctness of an existing CMMS on Kubernetes where the EDP simulators use
 * [SyntheticGeneratorEventQuery] with [SyntheticGenerationSpecs.SYNTHETIC_POPULATION_SPEC_LARGE].
 * The computation composition is using ACDP by assumption.

nit: document that this assuming that there's a Reporting system connected to the CMMS

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 134 at r1 (raw file):

          listEventGroupsRequest {
            parent = measurementConsumerName
            pageSize = 1000

nit: technically you still should be looping through to exhaust all pages regardless of what page size you set rather than relying on the fact that our current impl will give you the results

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 160 at r1 (raw file):

    try {
      return reportingSetsClient
        .withAuthenticationKey(apiAuthenticationKey)

The API key is only for the Kingdom public API.

Code quote:

.withAuthenticationKey(apiAuthenticationKey)

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 234 at r1 (raw file):

        Report.State.UNRECOGNIZED,
        Report.State.STATE_UNSPECIFIED -> delay(5000)
      }

Add some delay here so we're not polling as fast as possible.

---

src/test/kotlin/org/wfanet/measurement/integration/k8s/EmptyClusterCorrectnessTest.kt line 179 at r1 (raw file):

    override val reportingTestHarness: ReportingUserSimulator?
      get() = null

So this will only work when run on cloud? I assume we're hitting some GHA limit if we try to do this in a local cluster?

Code quote:

null

[tristanvuong2021]

Reviewable status: 6 of 19 files reviewed, 4 unresolved discussions (waiting on @SanjayVas)

---

.github/workflows/run-k8s-tests.yml line 55 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

I went ahead and added the variable for all 3 environments

Done.

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 160 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

The API key is only for the Kingdom public API.

Done.

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 234 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Add some delay here so we're not polling as fast as possible.

Done.

---

src/test/kotlin/org/wfanet/measurement/integration/k8s/EmptyClusterCorrectnessTest.kt line 179 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

So this will only work when run on cloud? I assume we're hitting some GHA limit if we try to do this in a local cluster?

Done.

[SanjayVas]

Reviewed 13 of 13 files at r2, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @tristanvuong2021)

---

src/main/k8s/local/testing/empty_encryption_key_pair_config.textproto line 1 at r2 (raw file):

# proto-file: wfa/measurement/config/reporting/encryption_key_pair_config.proto

This file already exists in the parent directory. If you need it from here, you can just add it to an exports_files declaration.

---

src/main/k8s/local/testing/mc_config_kustomization.yaml line 1 at r2 (raw file):

# Copyright 2024 The Cross-Media Measurement Authors

Same here re: existing in the parent package.

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 133 at r2 (raw file):

              listEventGroupsRequest {
                parent = measurementConsumerName
                pageSize = 1000

nit: just use the default page size

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 227 at r2 (raw file):

        Report.State.RUNNING,
        Report.State.UNRECOGNIZED,
        Report.State.STATE_UNSPECIFIED -> delay(5000)

nit: log that we're checking again in 5s like we do in the MC simulator

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 227 at r2 (raw file):

        Report.State.RUNNING,
        Report.State.UNRECOGNIZED,
        Report.State.STATE_UNSPECIFIED -> delay(5000)

nit: Consider using exponential backoff with a max delay like we do in the MC simulator. This will give us faster results for reports that complete sooner, but eventually go to a longer poll if the report is taking longer

[tristanvuong2021]

Reviewable status: 15 of 20 files reviewed, 2 unresolved discussions (waiting on @SanjayVas)

---

src/main/k8s/local/testing/empty_encryption_key_pair_config.textproto line 1 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

This file already exists in the parent directory. If you need it from here, you can just add it to an exports_files declaration.

I can't get this to work. Perhaps this is the reason why the db_creds_kustomization.yaml file is also an exact copy?

---

src/main/k8s/local/testing/mc_config_kustomization.yaml line 1 at r2 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Same here re: existing in the parent package.

Done.

[SanjayVas]

Reviewed 3 of 5 files at r3, all commit messages.
Reviewable status: 18 of 20 files reviewed, all discussions resolved

---

src/main/k8s/local/testing/empty_encryption_key_pair_config.textproto line 1 at r2 (raw file):

Previously, tristanvuong2021 (Tristan Vuong) wrote…

I can't get this to work. Perhaps this is the reason why the db_creds_kustomization.yaml file is also an exact copy?

Oh, I think I remember the issue. Kustomize is sensitive to exact file paths and doesn't even link symlinks. Therefore you will indeed need a copy :(

[SanjayVas]

Reviewed 4 of 4 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021)

[stevenwarejones]

Reviewed 6 of 11 files at r1, 9 of 13 files at r2, 1 of 5 files at r3, 4 of 4 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @tristanvuong2021)

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 17 at r4 (raw file):

 */

package org.wfanet.measurement.loadtest.reporting

this isn't just for loadtesting, right? shouldn't it be under org.wfanet.measurement.reporting.testing?

---

src/main/k8s/local/testing/BUILD.bazel line 73 at r4 (raw file):

kustomization_dir(
    name = "config_files_for_panel_match",

panel_match_config?

[SanjayVas]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @stevenwarejones)

---

src/main/k8s/local/testing/BUILD.bazel line 73 at r4 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

panel_match_config?

No. The config_files part of the name is referring to the ConfigMap named config-files.

---

src/main/kotlin/org/wfanet/measurement/loadtest/reporting/ReportingUserSimulator.kt line 17 at r4 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

this isn't just for loadtesting, right? shouldn't it be under org.wfanet.measurement.reporting.testing?

This is true of the entire loadtest package. It's an old mistake that has yet to be rectified.

[stevenwarejones]

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tristanvuong2021)

[tristanvuong2021]

Dismissed @stevenwarejones from a discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @tristanvuong2021)