Add executable binary classes and docker images for HMSS.

[renjiezh]

No description provided.

[wfa-reviewable]

This change is 

[ple13]

Reviewed 46 of 46 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @renjiezh and @SanjayVas)

---

src/main/k8s/kingdom.cue line 65 at r1 (raw file):

	_hmssRfEnableFlag:                       "--enable-hmss-for-rf"
	_hmssReachEnableFlag:                    "--enable-hmss-for-reach"
	_hmssProtocolConfigConfig:               "--hmss-protocol-config-config=/var/run/secrets/files/hmss_protocol_config_config.textproto"

nit: can we use the same format as in other lines?

Code quote:

	_hmssRfEnableFlag:                       "--enable-hmss-for-rf"
	_hmssReachEnableFlag:                    "--enable-hmss-for-reach"
	_hmssProtocolConfigConfig:               "--hmss-protocol-config-config=/var/run/secrets/files/hmss_protocol_config_config.textproto"

---

src/main/kotlin/org/wfanet/measurement/duchy/deploy/common/daemon/herald/HeraldDaemon.kt line 90 at r1 (raw file):

    description = ["The key encryption key file (binary format) used for private key store."],
  )
  var keyEncryptionKeyTinkFile: File? = null

nit: Do we need private set to make it read-only?

---

src/main/kotlin/org/wfanet/measurement/duchy/deploy/common/daemon/mill/shareshuffle/HonestMajorityShareShuffleMillDaemon.kt line 140 at r1 (raw file):

      flags.csCertificateDerFile.inputStream().use { input -> readCertificate(input) }
    val csCertificate = Certificate(flags.csCertificateName, csX509Certificate)
    // TODO: Read from a KMS-encrypted store instead.

nit: use the correct format for TODO (See

cross-media-measurement/docs/code-style.md

Line 167 in bb94176

### Format

).

[SanjayVas]

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

---

src/main/k8s/duchy.cue line 133 at r1 (raw file):

						_kingdom_system_api_target_flag,
						_kingdom_system_api_cert_host_flag,
						if (_duchyKeyEncryptionKeyFile != _|_) {_duchyKeyEncryptionKeyFileFlag},

This conditional doesn't make sense, as this field appears to be required.

---

src/main/kotlin/org/wfanet/measurement/duchy/deploy/common/daemon/herald/HeraldDaemon.kt line 146 at r1 (raw file):

  val heraldId = System.getenv("HOSTNAME")

  // TODO(@renjiez): Use real PrivateKeyStore when enabling HMSS.

Why was the TODO dropped? At what point pre-launch are we going to swap this over to a real KMS client? IMO at this point, FakeKmsClient should only be used from ForwardedStorageHeraldDaemon.

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 106 at r1 (raw file):

  private val noiseMechanisms: List<NoiseMechanism>,
  private val reachOnlyLlV2Enabled: Boolean = false,
  private val reachAndFrequencyHmssEnabled: Boolean = false,

Is this just being extra cautious, e.g. giving us the flexibility to disable HMSS for the REACH Measurement type? I would assume that if HMSS works at all, it works for both measurement types. This is because it's a single protocol.

Note that this is different than reachOnlyLlv2Enabled, which controls whether the separate RO LLv2 protocol is used. That is to say it does not control whether LLv2 can be used for the REACH measurement type. As a result, we may want to use different naming and some extra documentation here.

[renjiezh]

Reviewable status: 43 of 46 files reviewed, 4 unresolved discussions (waiting on @ple13 and @SanjayVas)

---

src/main/k8s/duchy.cue line 133 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

This conditional doesn't make sense, as this field appears to be required.

It is required only for worker1 and worker2. Aggregator does not need it.

---

src/main/k8s/kingdom.cue line 65 at r1 (raw file):

Previously, ple13 (Phi) wrote…

nit: can we use the same format as in other lines?

Do you mean camelCase vs all_lowercase? We will stick to camelCase forward.

---

src/main/kotlin/org/wfanet/measurement/duchy/deploy/common/daemon/herald/HeraldDaemon.kt line 146 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Why was the TODO dropped? At what point pre-launch are we going to swap this over to a real KMS client? IMO at this point, FakeKmsClient should only be used from ForwardedStorageHeraldDaemon.

Added it back. Will turn back to kms after reach-only HMSS.

---

src/main/kotlin/org/wfanet/measurement/kingdom/service/api/v2alpha/MeasurementsService.kt line 106 at r1 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Is this just being extra cautious, e.g. giving us the flexibility to disable HMSS for the REACH Measurement type? I would assume that if HMSS works at all, it works for both measurement types. This is because it's a single protocol.

Note that this is different than reachOnlyLlv2Enabled, which controls whether the separate RO LLv2 protocol is used. That is to say it does not control whether LLv2 can be used for the REACH measurement type. As a result, we may want to use different naming and some extra documentation here.

We are implementing reach-only HMSS now. The current HMSS is primarily targeting R/F though it works for reach-only (with too much noise). I leave this flexibility for now. Once reach-only implemented, I will merge these two options.
A TODO added.

[SanjayVas]

Reviewed 7 of 46 files at r1, 1 of 3 files at r2, all commit messages.
Reviewable status: 44 of 46 files reviewed, 6 unresolved discussions (waiting on @ple13 and @renjiezh)

---

src/main/k8s/duchy.cue line 133 at r1 (raw file):

Previously, renjiezh wrote…

It is required only for worker1 and worker2. Aggregator does not need it.

What I mean is that the field isn't listed as optional, and is also used in a another non-optional field.

You'll want to declare it with a question mark to indicate it's optional, and then do the same for the derived flag field (wrapping it in a conditional).

---

src/main/k8s/dev/duchy_eks.cue line 42 at r2 (raw file):

	requests: {
		cpu:    "25m"
		memory: "1024Mi"

Why are these values different on EKS vs. GKE? Is it that S3 client takes different memory than GCS client?

---

src/main/k8s/dev/duchy_gke.cue line 49 at r2 (raw file):

	}
}
#HeraldMaxHeapSize:        "400M"

I assume this memory increase is due to having a storage client?

---

src/main/k8s/dev/duchy_gke.cue line 59 at r2 (raw file):

	}
}
#MillMaxHeapSize:                 "5G"

This seems quite significant. Is this because of the HMSS mill needing the VID index map? Can we instead set the HMSS and LLv2 mills independently?

---

src/main/k8s/dev/duchy_gke.cue line 70 at r2 (raw file):

	}
}
#FulfillmentMaxHeapSize: "350M"

Why did this get so much bigger? Is it the VID index map thing again?

[renjiezh]

Reviewable status: 46 of 50 files reviewed, 3 unresolved discussions (waiting on @ple13 and @SanjayVas)

---

src/main/k8s/duchy.cue line 42 at r5 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

You shouldn't need the conditional here since it's setting an optional field from an optional field.

Done.

---

src/main/k8s/dev/duchy_eks.cue line 42 at r5 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

Incorrect units. K8s uses "Mi" for mebibyte, whereas JVM flags use "M".

Done.
Good to know. Thanks!

---

src/main/kotlin/org/wfanet/measurement/duchy/deploy/common/daemon/mill/shareshuffle/HonestMajorityShareShuffleMillFlags.kt line 28 at r5 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

nit: is it worth factoring out the common Mill flags?

Added a TODO for that.

[ple13]

Reviewed 2 of 3 files at r2, 10 of 11 files at r3, 2 of 2 files at r4, 2 of 2 files at r5, 4 of 4 files at r6, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @SanjayVas)

[SanjayVas]

Reviewed 5 of 46 files at r1, 4 of 4 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @stevenwarejones)

[stevenwarejones]

Reviewed 34 of 46 files at r1, 2 of 3 files at r2, 7 of 11 files at r3, 1 of 2 files at r4, 2 of 2 files at r5, 4 of 4 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @renjiezh)

---

src/main/kotlin/org/wfanet/measurement/duchy/deploy/common/daemon/mill/shareshuffle/HonestMajorityShareShuffleMillDaemon.kt line 139 at r6 (raw file):

    // This will be the name of the pod when deployed to Kubernetes. Note that the millId is
    // included in mill logs to help debugging.
    val millId = System.getenv("HOSTNAME")

Are there any scenarios where someone would want to override this? eg make this a flag with a default value you have here?

[SanjayVas]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/duchy/deploy/common/daemon/mill/shareshuffle/HonestMajorityShareShuffleMillDaemon.kt line 139 at r6 (raw file):

Previously, stevenwarejones (Steven Ware Jones) wrote…

Are there any scenarios where someone would want to override this? eg make this a flag with a default value you have here?

This is copied from the existing Mill code. I left this same comment on one of Matthew's PRs requesting a TODO to convert this to a command line option.

[renjiezh]

Reviewable status: 49 of 50 files reviewed, 1 unresolved discussion (waiting on @ple13, @SanjayVas, and @stevenwarejones)

---

src/main/kotlin/org/wfanet/measurement/duchy/deploy/common/daemon/mill/shareshuffle/HonestMajorityShareShuffleMillDaemon.kt line 139 at r6 (raw file):

Previously, SanjayVas (Sanjay Vasandani) wrote…

This is copied from the existing Mill code. I left this same comment on one of Matthew's PRs requesting a TODO to convert this to a command line option.

Added a TODO for it.
FYI, while using kubernetes, by default HOSTNAME is the pod name.

[stevenwarejones]

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

[renjiezh]

Reviewed 34 of 46 files at r1, 2 of 3 files at r2, 6 of 11 files at r3, 1 of 2 files at r4, 2 of 2 files at r5, 4 of 4 files at r6, 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)

[stevenwarejones]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @renjiezh)