Increase usability of Makefile.override #32660

learnitall · 2024-05-21T23:20:13Z

Please ensure your pull request adheres to the following guidelines:

For first time contributors, read Submitting a pull request
All code is covered by unit and/or runtime tests where feasible.
All commits contain a well written commit description including a title,
description and a Fixes: #XXX line if the commit addresses a particular
GitHub issue.
If your commit description contains a Fixes: <commit-id> tag, then
please add the commit author[s] as reviewer[s] to this issue.
All commits are signed off. See the section Developer’s Certificate of Origin
Provide a title or release-note blurb suitable for the release notes.
Are you a user of Cilium? Please add yourself to the Users doc
Thanks for contributing!

The goal of this PR is to make two slight adjustments to the way modifier flags, such as RACE, NOSTRIP and LOCKDEBUG, are handled, to increase the usability of Makefile.override in adjusting the build process for binaries and Docker images.

Currently, each 'modifier' that can be applied at build time is set through its own build argument within Makefiles and Docker images. For example, here is the flow for the NOSTRIP flag:

The user specifies NOSTRIP=1 on the command line when invoking make.
If the make target builds a docker image, such as with the target docker-images-all, the docker build argument named NOSTRIP is set to 1. This build argument is used to pass through the NOSTRIP flag to make invocations within the Dockerfile. See Makefile.docker and an example Dockerfile
If the make target builds a binary, the NOSTRIP flag is checked within Makefile.defs to determine if the -s and -w flags should be passed to go build via the GO_BUILD Makefile-level variable. When NOSTRIP contains some value, these flags are omitted. See Makefile.defs.

This process has a couple of limitations. First, each logical modifier that can be applied at build time requires its own docker build argument. This increases the number of layers in the produced docker image and the number of changes required to be made and backported to the image's Dockerfile. Second, the requirement for hardcoding a build argument for each modifier makes any additional build modification a user wants to add require changes within each target Dockerfile, each target binary's Makefile and within Makefile.docker.

This PR introduces the following changes to address this use case:

Docker build args for each logical modifier are replaced with a generalized MODIFIERS build arg, which is set within Makefile.docker. Existing modifiers are set automatically, with additional modifiers able to be passed in via an environment variable named ADDITIONAL_MODIFIERS. Existing modifiers, and new modifiers that are added to the tree, should be made available as environment variables to help ease documentation.
An include statement for Makefile.override was added within each binary's Makefile after the include of Makefile.defs. This allows for modifications to variables set within Makefile.defs through the use of Makefile.override. Makefile.override will have access to the flags that a user specifies with ADDITIONAL_MODIFIERS.

Here's an example of how this could be used. Say I'm a user with a fork of Cilium and I'm maintaining a new feature that needs to be enabled via a build tag named mycoolfeature. With the changes in this PR, this build flag can be toggled via an environment variable named MYCOOLFEATURE with the following Makefile.override:

ifneq ($(MYCOOLFEATURE),)
    GO_BUILD := $(GO_BUILD) -tags=mycoolfeature
endif

I could now build each Cilium docker image with my build tag with the following make invocation:

ADDITIONAL_MODIFIERS="MYCOOLFEATURE=1" make docker-images-all

Before this PR, the following changes would be needed:

--- a/Makefile.docker
+++ b/Makefile.docker
@@ -95,6 +95,7 @@ endif
                --build-arg RACE=${RACE}\
                --build-arg V=${V} \
                --build-arg LIBNETWORK_PLUGIN=${LIBNETWORK_PLUGIN} \
+               --build-arg MYCOOLFEATURE=${MYCOOLFEATURE} \
                --build-arg CILIUM_SHA=$(firstword $(GIT_VERSION)) \
                --build-arg OPERATOR_VARIANT=$(IMAGE_NAME) \
                --build-arg DEBUG_HOLD=$(DEBUG_HOLD) \

--- a/images/<RELEVANT DOCKERFILE>
+++ b/images/<RELEVANT DOCKERFILE>
@@ -40,6 +40,7 @@ ARG LOCKDEBUG
 ARG RACE
 ARG V
 ARG LIBNETWORK_PLUGIN
+ARG MYCOOLFEATURE
 
 WORKDIR /go/src/github.com/cilium/cilium
 
 RUN --mount=type=bind,readwrite,target=/go/src/github.com/cilium/cilium \
     --mount=type=cache,target=/root/.cache \
     --mount=type=cache,target=/go/pkg \
-    make GOARCH=${TARGETARCH} RACE=${RACE} NOSTRIP=${NOSTRIP} NOOPT=${NOOPT} LOCKDEBUG=${LOCKDEBUG} \
+    make GOARCH=${TARGETARCH} RACE=${RACE} NOSTRIP=${NOSTRIP} NOOPT=${NOOPT} LOCKDEBUG=${LOCKDEBUG} MYCOOLFEATURE=${MYCOOLFEATURE} \

index 364c60ad11..b73710d01b 100644
--- a/<RELEVANT BINARY MAKEFILE>
+++ b/<RELEVANT BINARY MAKEFILE>
@@ -5,6 +5,10 @@ ROOT_DIR := $(shell dirname "$(realpath $(lastword $(MAKEFILE_LIST)))")
 
include ${ROOT_DIR}/../Makefile.defs
 
+ ifneq ($(MYCOOLFEATURE),)
+     GO_BUILD := $(GO_BUILD) -tags mycoolfeature
+ endif

<!-- Enter the release note text here if needed or remove this section! -->

learnitall · 2024-05-22T15:47:07Z

/test

learnitall · 2024-05-24T13:30:20Z

To test the changes to the ci images workflow, I created a patch that added a pull_request trigger and replaced all references to pull_request_target in the workflow with pull_request. Full diff is below. This allowed for the changes in the workflow in this PR to be picked up by GitHub and executed.

Here's the link to the successful run: https://github.com/cilium/cilium/actions/runs/9194660121?pr=32660

diff --git a/.github/workflows/build-images-ci.yaml b/.github/workflows/build-images-ci.yaml
index 67287bb256..a1fdaea5a9 100644
--- a/.github/workflows/build-images-ci.yaml
+++ b/.github/workflows/build-images-ci.yaml
@@ -11,6 +11,7 @@ on:
     branches:
       - main
       - ft/main/**
+  pull_request:
 
   # If the cache was cleaned we should re-build the cache with the latest commit
   workflow_run:
@@ -140,7 +141,7 @@ jobs:
 
       # main branch pushes
       - name: CI Build ${{ matrix.name }}
-        if: ${{ github.event_name != 'pull_request_target' && !startsWith(github.ref_name, 'ft/') }}
+        if: ${{ github.event_name != 'pull_request' && !startsWith(github.ref_name, 'ft/') }}
         uses: docker/build-push-action@2cdde995de11925a030ce8070c3d77a52ffcf1c0 # v5.3.0
         id: docker_build_ci
         with:
@@ -160,7 +161,7 @@ jobs:
             OPERATOR_VARIANT=${{ matrix.name }}
 
       - name: CI race detection Build ${{ matrix.name }}
-        if: ${{ github.event_name != 'pull_request_target' && !startsWith(github.ref_name, 'ft/') }}
+        if: ${{ github.event_name != 'pull_request' && !startsWith(github.ref_name, 'ft/') }}
         uses: docker/build-push-action@2cdde995de11925a030ce8070c3d77a52ffcf1c0 # v5.3.0
         id: docker_build_ci_detect_race_condition
         with:
@@ -182,7 +183,7 @@ jobs:
             OPERATOR_VARIANT=${{ matrix.name }}
 
       - name: CI Unstripped Binaries Build ${{ matrix.name }}
-        if: ${{ github.event_name != 'pull_request_target' && !startsWith(github.ref_name, 'ft/') }}
+        if: ${{ github.event_name != 'pull_request' && !startsWith(github.ref_name, 'ft/') }}
         uses: docker/build-push-action@2cdde995de11925a030ce8070c3d77a52ffcf1c0 # v5.3.0
         id: docker_build_ci_unstripped
         with:
@@ -292,7 +293,7 @@ jobs:
 
       # PR or feature branch updates
       - name: CI Build ${{ matrix.name }}
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         uses: docker/build-push-action@2cdde995de11925a030ce8070c3d77a52ffcf1c0 # v5.3.0
         id: docker_build_ci_pr
         with:
@@ -308,7 +309,7 @@ jobs:
             OPERATOR_VARIANT=${{ matrix.name }}
 
       - name: CI race detection Build ${{ matrix.name }}
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         uses: docker/build-push-action@2cdde995de11925a030ce8070c3d77a52ffcf1c0 # v5.3.0
         id: docker_build_ci_pr_detect_race_condition
         with:
@@ -326,7 +327,7 @@ jobs:
             OPERATOR_VARIANT=${{ matrix.name }}
 
       - name: CI Unstripped Binaries Build ${{ matrix.name }}
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         uses: docker/build-push-action@2cdde995de11925a030ce8070c3d77a52ffcf1c0 # v5.3.0
         id: docker_build_ci_pr_unstripped
         with:
@@ -343,14 +344,14 @@ jobs:
             OPERATOR_VARIANT=${{ matrix.name }}
 
       - name: Sign Container Images
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         run: |
           cosign sign -y quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci@${{ steps.docker_build_ci_pr.outputs.digest }}
           cosign sign -y quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci@${{ steps.docker_build_ci_pr_detect_race_condition.outputs.digest }}
           cosign sign -y quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci@${{ steps.docker_build_ci_pr_unstripped.outputs.digest }}
 
       - name: Generate SBOM
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         uses: anchore/sbom-action@7ccf588e3cf3cc2611714c2eeae48550fbc17552 # v0.15.11
         with:
           artifact-name: sbom_ci_pr_${{ matrix.name }}_${{ steps.tag.outputs.tag }}.spdx.json
@@ -358,7 +359,7 @@ jobs:
           image: quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci:${{ steps.tag.outputs.tag }}
 
       - name: Generate SBOM (race)
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         uses: anchore/sbom-action@7ccf588e3cf3cc2611714c2eeae48550fbc17552 # v0.15.11
         with:
           artifact-name: sbom_ci_pr_race_${{ matrix.name }}_${{ steps.tag.outputs.tag }}.spdx.json
@@ -366,7 +367,7 @@ jobs:
           image: quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci:${{ steps.tag.outputs.tag }}-race
 
       - name: Generate SBOM (unstripped)
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         uses: anchore/sbom-action@7ccf588e3cf3cc2611714c2eeae48550fbc17552 # v0.15.11
         with:
           artifact-name: sbom_ci_pr_unstripped_${{ matrix.name }}_${{ steps.tag.outputs.tag }}.spdx.json
@@ -374,14 +375,14 @@ jobs:
           image: quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci:${{ steps.tag.outputs.tag }}-unstripped
 
       - name: Attach SBOM to Container Images
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         run: |
           cosign attach sbom --sbom sbom_ci_pr_${{ matrix.name }}_${{ steps.tag.outputs.tag }}.spdx.json quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci@${{ steps.docker_build_ci_pr.outputs.digest }}
           cosign attach sbom --sbom sbom_ci_pr_race_${{ matrix.name }}_${{ steps.tag.outputs.tag }}.spdx.json quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci@${{ steps.docker_build_ci_pr_detect_race_condition.outputs.digest }}
           cosign attach sbom --sbom sbom_ci_pr_unstripped_${{ matrix.name }}_${{ steps.tag.outputs.tag }}.spdx.json quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci@${{ steps.docker_build_ci_pr_unstripped.outputs.digest }}
 
       - name: Sign SBOM Images
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         run: |
           docker_build_ci_pr_digest="${{ steps.docker_build_ci_pr.outputs.digest }}"
           image_name="quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci:${docker_build_ci_pr_digest/:/-}.sbom"
@@ -399,7 +400,7 @@ jobs:
           cosign sign -y "quay.io/${{ env.QUAY_ORGANIZATION_DEV }}/${{ matrix.name }}-ci@${docker_build_ci_pr_unstripped_sbom_digest}"
 
       - name: CI Image Releases digests
-        if: ${{ github.event_name == 'pull_request_target' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
+        if: ${{ github.event_name == 'pull_request' || (github.event_name == 'push' && startsWith(github.ref_name, 'ft/')) }}
         shell: bash
         run: |
           mkdir -p image-digest/
@@ -417,7 +418,7 @@ jobs:
 
       # Store docker's golang's cache build locally only on the main branch
       - name: Store ${{ matrix.name }} Golang cache build locally
-        if: ${{ github.event_name != 'pull_request_target' && steps.cache.outputs.cache-hit != 'true' && github.ref_name == github.event.repository.default_branch }}
+        if: ${{ github.event_name != 'pull_request' && steps.cache.outputs.cache-hit != 'true' && github.ref_name == github.event.repository.default_branch }}
         uses: docker/build-push-action@2cdde995de11925a030ce8070c3d77a52ffcf1c0 # v5.3.0
         with:
           provenance: false
@@ -430,7 +431,7 @@ jobs:
 
       # Store docker's golang's cache build locally only on the main branch
       - name: Store ${{ matrix.name }} Golang cache in GitHub cache path
-        if: ${{ github.event_name != 'pull_request_target' && steps.cache.outputs.cache-hit != 'true' && github.ref_name == github.event.repository.default_branch }}
+        if: ${{ github.event_name != 'pull_request' && steps.cache.outputs.cache-hit != 'true' && github.ref_name == github.event.repository.default_branch }}
         shell: bash
         run: |
           mkdir -p /tmp/.cache/${{ matrix.name }}/

kaworu

Hubble Relay build changes LGTM

This commit replaces the NOSTRIP, NOOPT, LOCKDEBUG, RACE, V and LIBNETWORK_PLUGIN docker build args with a single, generic build arg named "MODIFIERS". This allows for arbitrary flags to be passed to make when building a docker image as well as removes the need for modifications to dockerfiles when a new build-time modifier is added. One example use case is using `Makefile.overrides` to define a new flag that can be passed to make when building docker images. The new flag could enable appending values to the MODIFIERS build argument, which would allow the propagation of configuration variables down to make invocations used to build binaries within a Dockerfile. Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>

This commit adds a new environment variable to the docker-specific aspects of the Cilium Makefiles named `ADDITIONAL_MODIFIERS`. This environment variable can be used to modify the `MODIFIERS` docker build arg, adding in any extra values that haven't previously been specified via a preset, such as `RACE` or `NOSTRIP`. Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>

make: Add include to Makefile.override in binary Makefiles This commit adds an include statement for Makefile.override in Makefiles specific to building Cilium's go binaries. Makefile.override is included in the top-level Makefile as a method for optionally overriding variables, however it is not included in any of these binary-specific Makefiles. This means that the ability to override variables is only available for targets in the top-level Makefile, preventing use cases where overriding variables used in these binary-specific Makefiles can be useful. As an example, this commit would allow one to override the GO variable to specify a specific go binary to use in order to build a target. Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>

learnitall · 2024-06-10T16:39:34Z

/test

learnitall · 2024-06-10T16:40:15Z

Rebased and ran a test commit to check for accuracy. Here's the passing run: https://github.com/cilium/cilium/actions/runs/9451756541?pr=32660.

learnitall · 2024-06-11T17:39:00Z

/test

giorio94 · 2024-06-13T07:28:40Z

images/operator/Dockerfile

-ARG NOSTRIP
-ARG NOOPT
-ARG LOCKDEBUG
-ARG RACE
+# OPERATOR_VARIANT determines the target cloud provider to build the operator for.
 ARG OPERATOR_VARIANT
+# MODIFIERS are extra arguments to be passed to make at build time.
+ARG MODIFIERS


@learnitall The changes here are not reflected in the make call below. I assume that this would need to be fixed, right?

Yep, thank you for opening a fix.

In PR cilium#32660, the MODIFIERS build argument was added as a replacement for individual build arguments, one of which was NOSTRIP. There was an uncaught usage of NOSTRIP in cilium's Dockerfile, which has caused the NOSTRIP modifier to become a no-op. This commit fixes this usage, allowing for NOSTRIP to be used again. Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>

In PR #32660, the MODIFIERS build argument was added as a replacement for individual build arguments, one of which was NOSTRIP. There was an uncaught usage of NOSTRIP in cilium's Dockerfile, which has caused the NOSTRIP modifier to become a no-op. This commit fixes this usage, allowing for NOSTRIP to be used again. Signed-off-by: Ryan Drew <ryan.drew@isovalent.com>

maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 21, 2024

learnitall force-pushed the pr/learnitall/generalize-modifiers branch from 3e1a28d to d5265f6 Compare May 22, 2024 15:56

learnitall added the release-note/misc This PR makes changes that have no direct user impact. label May 22, 2024

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 22, 2024

learnitall added area/build Anything to do with the build, more general than area/CI dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels May 22, 2024

maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label May 22, 2024

learnitall added the area/CI Continuous Integration testing issue or flake label May 22, 2024

learnitall force-pushed the pr/learnitall/generalize-modifiers branch from d5265f6 to 78ae62d Compare May 22, 2024 16:01

learnitall force-pushed the pr/learnitall/generalize-modifiers branch from 78ae62d to 8c11dea Compare May 24, 2024 13:32

learnitall marked this pull request as ready for review May 24, 2024 13:33

learnitall requested review from a team as code owners May 24, 2024 13:33

learnitall requested review from pippolo84, kaworu, borkmann, YutaroHayakawa, asauber and viktor-kurchenko May 24, 2024 13:33

learnitall added needs-backport/1.15 affects/v1.15 This issue affects v1.15 branch labels May 24, 2024

YutaroHayakawa approved these changes May 28, 2024

View reviewed changes

kaworu approved these changes May 29, 2024

View reviewed changes

borkmann added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 3, 2024

borkmann approved these changes Jun 3, 2024

View reviewed changes

learnitall force-pushed the pr/learnitall/generalize-modifiers branch from 8c11dea to 26d4481 Compare June 10, 2024 16:04

learnitall added 3 commits June 10, 2024 10:07

learnitall force-pushed the pr/learnitall/generalize-modifiers branch from 26d4481 to 22ecbd6 Compare June 10, 2024 16:07

learnitall force-pushed the pr/learnitall/generalize-modifiers branch from 22ecbd6 to 07f5c0b Compare June 10, 2024 16:40

learnitall removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 11, 2024

maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 11, 2024

dylandreimerink added this pull request to the merge queue Jun 12, 2024

Merged via the queue into main with commit 811cb7f Jun 12, 2024

dylandreimerink deleted the pr/learnitall/generalize-modifiers branch June 12, 2024 08:25

giorio94 mentioned this pull request Jun 12, 2024

v1.15 Backports 2024-06-12 #33098

Merged

4 tasks

giorio94 added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 labels Jun 12, 2024

giorio94 reviewed Jun 13, 2024

View reviewed changes

giorio94 mentioned this pull request Jun 13, 2024

Miscellaneous fixes in the usage of Makefile.override and build modifiers #33129

Merged

github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Jun 19, 2024

This was referenced Jul 10, 2024

Prepare for release v1.15.7 #33712

Closed

Prepare for release v1.15.7 #33713

Merged

learnitall mentioned this pull request Oct 9, 2024

docker: Fix NOSTRIP option in cilium/Dockerfile #35326

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Increase usability of Makefile.override #32660

Increase usability of Makefile.override #32660

Uh oh!

learnitall commented May 21, 2024

Uh oh!

learnitall commented May 22, 2024

Uh oh!

learnitall commented May 24, 2024

Uh oh!

kaworu left a comment

Uh oh!

learnitall commented Jun 10, 2024

Uh oh!

learnitall commented Jun 10, 2024

Uh oh!

learnitall commented Jun 11, 2024

Uh oh!

giorio94 Jun 13, 2024

Uh oh!

learnitall Jun 14, 2024

Uh oh!

Uh oh!

Increase usability of Makefile.override #32660

Increase usability of Makefile.override #32660

Uh oh!

Conversation

learnitall commented May 21, 2024

Uh oh!

learnitall commented May 22, 2024

Uh oh!

learnitall commented May 24, 2024

Uh oh!

kaworu left a comment

Choose a reason for hiding this comment

Uh oh!

learnitall commented Jun 10, 2024

Uh oh!

learnitall commented Jun 10, 2024

Uh oh!

learnitall commented Jun 11, 2024

Uh oh!

giorio94 Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

learnitall Jun 14, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!