Rewrite L4 healthchecks creation and deletion #1705

cezarygerard · 2022-05-05T15:40:35Z

create a common singleton like struct fot l4 health checks
new struct holds mutex for common resources
delete shared healtcheck firewall rules safely

without proper guarding of shared healtcheck firewall rules, the NetLB controller may delete the ILB healthcheck firewall or vice versa

k8s-ci-robot · 2022-05-05T15:40:43Z

Hi @cezarygerard. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cezarygerard · 2022-05-05T15:44:02Z

/assign @bowei

pkg/healthchecks/healthchecks_l4.go

pkg/loadbalancers/interfaces.go

pkg/loadbalancers/l4netlb.go

pkg/loadbalancers/l4netlb_test.go

bowei · 2022-05-05T22:15:09Z

/ok-to-test

bowei · 2022-05-05T22:15:19Z

/assign

cezarygerard · 2022-05-05T23:13:28Z

@bowei I have just answered all questions from Katarzyna.
Please review this code as we need this before we go GA with RBS feature

bowei

Biggest question is why introduce a singleton? Seems like it's mostly to reduce verbosity?

bowei · 2022-05-05T22:15:55Z

cmd/glbc/main.go

@@ -19,6 +19,7 @@ package main
 import (
 	"context"
 	"fmt"
+	"k8s.io/ingress-gce/pkg/healthchecks"


Put non-golang libs on a separate block for imports

bowei · 2022-05-05T22:16:18Z

hack/run-local-glbc.sh

@@ -43,5 +43,7 @@ ${GLBC} \
    --running-in-cluster=false \
    --logtostderr --v=${V} \
    --config-file-path=${GCECONF} \
+    --run-l4-controller \


Put this in a separate commit

undone the change

bowei · 2022-05-05T22:17:00Z

pkg/firewalls/firewalls_l4.go

@@ -104,8 +104,8 @@ func EnsureL4FirewallRuleDeleted(cloud *gce.Cloud, fwName string) error {
 }

 func firewallRuleEqual(a, b *compute.Firewall) bool {
-	return a.Description == b.Description &&
-		len(a.Allowed) == 1 && len(a.Allowed) == len(b.Allowed) &&
+	// let's skip description not to trigger flood of updates


// Don't compare the "description" field.

done

changed the logic a little bit for non-shared health checks and firewalls according to #1705 (comment)

bowei · 2022-05-05T22:18:56Z

pkg/firewalls/firewalls_l4.go

-	return a.Description == b.Description &&
-		len(a.Allowed) == 1 && len(a.Allowed) == len(b.Allowed) &&
+	// let's skip description not to trigger flood of updates
+	return len(a.Allowed) == 1 && len(a.Allowed) == len(b.Allowed) &&


put one predicate per line

done

changed the logic a little bit for non-shared health checks and firewalls according to #1705 (comment)

pkg/healthchecks/healthchecks_l4.go

bowei · 2022-05-05T23:53:04Z

pkg/healthchecks/healthchecks_l4.go

+}
+
+func Initialize(cloud *gce.Cloud, recorderFactory events.RecorderProducer) {
+	instance = &l4HealthChecks{


Doesn't this need to take the mutex?

Added a mutex guard,
btw see my comment on singleton pattern

bowei · 2022-05-06T06:17:30Z

pkg/healthchecks/healthchecks_l4.go

@@ -42,44 +47,111 @@ const (
 	gceHcUnhealthyThreshold = int64(3)
 )

-// EnsureL4HealthCheck creates a new HTTP health check for an L4 LoadBalancer service, based on the parameters provided.
+var instance *l4HealthChecks


Can you explain the rationale behind adding a singleton. Seems like it mostly just removes the Cloud and recorder argument? If we find that a problem because it's too verbose -- maybe just put those in a single struct and pass that as an argument instead of having to deal with the complexities of having a Singleton?

It is not about cloud and recorder args.

We have a real bug in healthcheck deletion.

Deletion of shared health check tries to remove firewall rule for health check
and the healthcheck from ILB and NetLB use the same firewall rule

Checking if healthcheck-under-deletion is in use is not enough (deleting NetLB Healthcheck will not fail if ILB Healthcheck is in use)
cross-checking firewalls is still subject to race conditions (one healthcheck is deleted, the other is added ad the same time)

so we need a way to safely healthchecks using the same mutex object across the NetLB controller and ILB controller.

Creating a common l4HealthChecks struct makes a good place for cross-checking healthchecks and simplifies code in NetLB controller and ILB controller.

Now the open question whether it is worth to create the singleton

it make initialisation in l4.go and l4netlb.go much easier
but on the other hand the tests setups are more complicated

maybe it is enough to create an instance in main and pass it via context, just like any other single instance struct in this code

I have tried to sketch the old way for dependency injection: from context down to controllers na handlers
cezarygerard#3

it does not seem much simpler

bowei · 2022-05-06T06:18:21Z

pkg/healthchecks/healthchecks_l4.go

+	}
+}
+
+func Fake(cloud *gce.Cloud, recorderFactory events.RecorderProducer) *l4HealthChecks {


This seems like it doesn't actually create a fake?

You are right,
My rationale was to name it Fake so it is clearly only for test purposes.

kl52752 · 2022-05-06T06:21:40Z

pkg/firewalls/firewalls_l4.go

@@ -104,8 +102,8 @@ func EnsureL4FirewallRuleDeleted(cloud *gce.Cloud, fwName string) error {
 }

 func firewallRuleEqual(a, b *compute.Firewall) bool {
-	return a.Description == b.Description &&
-		len(a.Allowed) == 1 && len(a.Allowed) == len(b.Allowed) &&
+	// let's skip description not to trigger flood of updates


I'm not sure if we wants to do this. For non-shared HC description contains ServiceIP: ip, so if IP will change then the health check description will be obsolete, it probably is needed only for debugging purposes but still it might be confusing.

I have changed this logic, for non shared health checks

pkg/healthchecks/healthchecks_l4.go

pkg/loadbalancers/l4netlb_test.go

bowei · 2022-05-09T07:35:05Z

Can you make sure the Git commits follow good conventions (e.g. have one line + description describing the bug being fixed etc.)

pkg/loadbalancers/l4netlb_test.go

kl52752 · 2022-05-09T11:10:06Z

pkg/firewalls/firewalls_l4.go

-		len(a.Allowed) == 1 && len(a.Allowed) == len(b.Allowed) &&
+func firewallRuleEqual(a, b *compute.Firewall, skipDescription bool) bool {
+	fwrEqual := len(a.Allowed) == 1 &&
+		len(a.Allowed) == len(b.Allowed) &&


maybe instaed of creating new variable fwrEqual you can do :

.. previous check.. && (skipDescription || a.Description == b.Description)

refactored this whole func

kl52752 · 2022-05-09T11:16:25Z

pkg/healthchecks/healthchecks_l4.go

+}
+
+// GetL4 returns singleton instance, must be run after InitializeL4
+func GetL4() *l4HealthChecks {


why don't you put Init func here? (and make Init function not exported).
Checking instance == nil is cheap and then you don't have to worry that someone will call GetL4 before Init function.
I also think that it would be better to create this object inside context and keep it there (sinc all controllers have context and context is explicitly initialise before controllers are run so it will be safe) instead of this singleton pattern but this is just my opinion.

[minor] It's probably more "Go"ish to call this L4()

@kl52752

The l4HealthChecks needs recorder factory which is not easily accessible from l4.go and l4netlb.go

I have tried to create L4HealthChecks in context but I get import cycle error due to pkg/firewalls using the context

@bowei
Changed.

kl52752 · 2022-05-09T11:18:15Z

pkg/healthchecks/healthchecks_l4.go

+
+// EnsureL4HealthCheck creates a new HTTP health check for an L4 LoadBalancer service, and the firewall rule required
+// for the healthcheck. If healthcheck is shared (external traffic policy 'cluster') then firewall rules will be shared
+// regardless of scope param.


I think that this comment is not complete. I don't understand (without knowing the context) firewall rules will be shared regardless of scope param what is scope param? Just write that ILB creates Global HC and NetLB Regional and firewall rule is shared for both.

+1 we should be very explicit in our comments, otherwise it ends up with more questions than answers:

Suggest (if accurate)

EnsureL4HealthCheck and firewall rules exist for the L4 LoadBalancer Service. The healthcheck and firewall will be shared between different K8s Services for ExternalTrafficPolicy = Cluster, as the same configuration is used across all Services of this type. Firewall rules are always created at in the Global scope (vs Regional). This means that one Firewall rule is created for Services of different scope (Global vs Regional).

can we use this comment?

yes, used your comment

pkg/loadbalancers/l4netlb_test.go

cezarygerard · 2022-05-09T23:02:28Z

@bowei done

bowei · 2022-05-17T05:55:34Z

pkg/healthchecks/healthchecks_l4.go

+var (
+	// instance is a sinngleton instance, created by InitializeL4
+	instance *l4HealthChecks
+	// mutex for preventing multiple initialization


Let's call this instanceLock

// instanceLock to prevent duplicate initialization.

bowei

Make sure we have sufficient logging for debugging.

bowei · 2022-05-17T05:56:39Z

pkg/healthchecks/healthchecks_l4.go

+)
+
+type l4HealthChecks struct {
+	mutex           sync.Mutex


Can we document what this mutex is for? what is it used to protect?

done.

added comment
renamed to sharedResourcesLock as it was in previous implementation

bowei · 2022-05-17T06:05:49Z

pkg/healthchecks/healthchecks_l4.go

+
+// InitializeL4 creates singleton instance, must be run before GetL4() func
+func InitializeL4(cloud *gce.Cloud, recorderFactory events.RecorderProducer) {
+	if instance == nil {


if instance != nil { log.Error double init return }

Error, not warning?
ok, done error

pkg/healthchecks/healthchecks_l4.go

bowei · 2022-05-17T06:18:38Z

pkg/healthchecks/healthchecks_l4.go

 // If the healthcheck already exists, it is updated as needed.
-func EnsureL4HealthCheck(cloud *gce.Cloud, svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType) (string, string, int32, string, error) {
+func (l4hc *l4HealthChecks) EnsureL4HealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) (string, string, string, string, error) {


If it's not a huge change, we should change the return value to a struct with named fields.

type EnsureL4HealthCheckResult struct { HCLink string ... }

fixed

but to declare the return struct I must have moved the interfaces to the to the provider package instead of client package to avoid circular dependencies

bowei · 2022-05-17T06:19:10Z

pkg/healthchecks/healthchecks_l4.go

-		hcPath, hcPort = helpers.GetServiceHealthCheckPathPort(svc)
+	hcPath, hcPort := helpers.GetServiceHealthCheckPathPort(svc)
+	if sharedHC {
+		hcPath, hcPort = gce.GetNodesHealthCheckPath(), gce.GetNodesHealthCheckPort()


we should log.V(3).Infof or higher

bowei · 2022-05-17T06:32:48Z

pkg/healthchecks/healthchecks_l4.go

+	return "", nil
+}
+
+func (l4hc *l4HealthChecks) healthCheckFirewallSafeToDelete(hcName string, sharedHC bool, l4Type utils.L4LBType) (bool, error) {


we should name this func closer to what it indicates rather than the interpreted meaning.

isn't this just healthcheckExists()?

it is actually inversed

if healthcheck exists then the function will return false
also it always returns true for shared healthchecks

maybe: healthCheckFirewallInUse ?

but it still inverst entire logic

bowei · 2022-05-17T06:34:56Z

pkg/healthchecks/healthchecks_l4.go

+
+func (l4hc *l4HealthChecks) deleteFirewall(name string, svc *corev1.Service) error {
+	err := firewalls.EnsureL4FirewallRuleDeleted(l4hc.cloud, name)
+	if err != nil {


if err == nil { return nil }

log.Error the error

Also, we should have a comment saying why we ignore the FIrewallXPNError

should we use err.As() instead of the type case?

added comment

Log is written inside firewalls.EnsureL4FirewallRuleDeleted for XPN firewall rules errors

The type cast code has been moved from l4.go/l4netlb.go without changing the logic, therefore I am hesitant to change to errors.As(...)

bowei · 2022-05-17T06:37:15Z

pkg/loadbalancers/interfaces.go

+// L4HealthChecks defines methods for creating and deleting health checks (and their firewall rules) for l4 services
+type L4HealthChecks interface {
+	// EnsureL4HealthCheck creates health check (and firewall rule) for l4 service
+	EnsureL4HealthCheck(svc *corev1.Service, namer namer.L4ResourcesNamer, sharedHC bool, scope meta.KeyType, l4Type utils.L4LBType, nodeNames []string) (string, string, string, string, error)


see comment about this return type

fixed

but to declare the struct I must have moved the interfaces to the to the provider package instead of client package to avoid circular dependencies

bowei · 2022-05-17T06:38:53Z

pkg/loadbalancers/l4.go

+	err := firewalls.EnsureL4FirewallRuleDeleted(l.cloud, name)
+	if err != nil {
+		if fwErr, ok := err.(*firewalls.FirewallXPNError); ok {
+			l.recorder.Eventf(l.Service, corev1.EventTypeNormal, "XPN", fwErr.Message)


we should probably log info here

the info is logged in the firewalls.EnsureL4FirewallRuleDeleted if XPN error occurs

bowei · 2022-05-17T06:40:10Z

pkg/loadbalancers/l4netlb.go

-				result.Error = err
-			}
+			result.GCEResourceInError = resourceInError
+			result.Error = err


Do we need to break if we get an error from this loop?

no, customer might have updated the service from ExternalTrafficPolicy=Local to ExternalTrafficPolicy=Cluster, and now they are deleting the service.

We do not try to cleanup firewall rules during ExternalTrafficPolicy change, so we must cleanup here

even if shared HC deletion failed there stil may be non shared HC

can we add a comment describing why

added more comment line here
there is also full rationale in the comments just before the for loop

cezarygerard · 2022-05-17T18:08:35Z

@bowei
done and answered all comments

bowei · 2022-05-18T00:18:30Z

pkg/firewalls/firewalls_l4.go

+		}
+	}
+
+	srcAndTargetEqual := utils.EqualStringSets(a.SourceRanges, b.SourceRanges) &&


it's easier to maintain if we make this straight line code:

if ! utils.EqualStringSets() { return false } if ! utils.EqualStringSets() { return false } if ! skipDescription && a.Description != b.Description { return false } return true

bowei · 2022-05-18T00:20:33Z

pkg/healthchecks/healthchecks_l4.go

+	// instance is a singleton instance, created by InitializeL4
+	instance *l4HealthChecks
+	// instanceLock to prevent duplicate initialization.
+	instanceLock = &sync.Mutex{}


go style puts the mutex in the same block as the protected var at the top:

struct { lock var var nonProtectedVar ... }

bowei · 2022-05-18T00:22:01Z

pkg/healthchecks/healthchecks_l4.go

+)
+
+type l4HealthChecks struct {
+	// Lock access to shared resources - node healthcecks and their firewall rules


We should be more explicit:

// sharedResourceLock serializes operations on the healthcheck and firewall
// resources shared across multiple Services.

bowei · 2022-05-18T00:22:32Z

pkg/healthchecks/healthchecks_l4.go

+		return
+	}
+
+	instanceLock.Lock()


You need to put this first, no? the instance != nil needs to be guarded.

Usually the singleton has 2 checks,

to only lock the mutex if the instance is not initialised
and

to make sure that instance was not initialised between first check and lock acquisition.

The code used to look like this:

if instance == nil { initLock.Lock() defer initLock.Unlock() if instance == nil { instance = &l4HealthChecks{ cloud: cloud, recorderFactory: recorderFactory, } } }

in the comnent #1705 (comment)
you asked me to log double init attempts

so I changed the above code into

if instance != nil { klog.Error("Multiple L4 Healthchecks initialization attempts") return } instanceLock.Lock() defer instanceLock.Unlock() if instance == nil { instance = &l4HealthChecks{ cloud: cloud, recorderFactory: recorderFactory, } klog.V(3).Infof("Initialized L4 Healthchecks") }

which is equivalent of double checked locking.

I can just lock out entire function,

instanceLock.Lock() defer instanceLock.Unlock() if instance != nil { klog.Error("Multiple L4 Healthchecks initialization attempts") return } instance = &l4HealthChecks{ cloud: cloud, recorderFactory: recorderFactory, } klog.V(3).Infof("Initialized L4 Healthchecks")

as InitializeL4 is only called once (not in every healthchecks.GetL4() call )

are you ok with the last option?

bowei · 2022-05-18T00:22:59Z

pkg/healthchecks/healthchecks_l4.go

+	instanceLock.Lock()
+	defer instanceLock.Unlock()
+
+	if instance == nil {


you don't need this if any more

please see the previous comment

bowei · 2022-05-18T00:38:49Z

pkg/healthchecks/healthchecks_l4.go

+	if err != nil {
+		return false, fmt.Errorf("Failed to create composite key for healthcheck %s - %w", hcName, err)
+	}
+	_, err = composite.GetHealthCheck(l4hc.cloud, key, meta.VersionGA)


Don't we need to take the lock here to avoid something creating in parallel to the deletion?
This has a lock read unlock; A ; lock write unlock issue -- there might be some operation occurring at A that is a race.

TBH, I don't get this race issue here
Could you please elaborate?

please note that EnsureL4HealthCheck and DeleteHealthCheck execute
l4hc.sharedResourcesLock.Lock()
very early before any API calls

so
between healthCheckFirewallSafeToDelete call and actual deletion, the sharedResourcesLock is still possessed by the same thread

cezarygerard · 2022-05-26T15:56:54Z

@bowei @kl52752
I have answered all your comments

- create a common singleton-like struct fot l4 health checks - new struct holds mutex for common resources (healthchecks and their firewall rules used for - [bugfix] delete shared healtcheck firewall rules safely - cross-check between ILB and NLB healthchecks is firewall rules are in use Logging New log line idicating firewall rule not deleted due to cross-check: "Failed to delete health check firewall rule %s: health check in use." Testing - healthcheck management is mostly covered (by existing tests), they required little update. - added test cases for sahred firewall rule deletion(lack of), named TestHealthCheckFirewallDeletionWithILB and TestHealthCheckFirewallDeletionWithNetLB - run test manual tests

pkg/healthchecks/healthchecks_l4.go

cezarygerard · 2022-05-30T16:54:44Z

/test pull-ingress-gce-test

EnsureL4HealthCheck: replace lengthy return value list in with named struct Improve firewall rule comparison Added debug logs Improved go fmt Renamed function names And many more small ones

panslava · 2022-06-01T09:14:18Z

pkg/healthchecks/interfaces.go

+	HCName             string
+	HCLink             string
+	HCFirewallRuleName string
+	GceResourceInError string


Let's rename this to GCEResourceInError

bowei · 2022-06-01T19:15:33Z

/approve

kl52752 · 2022-06-02T09:54:30Z

/lgtm

k8s-ci-robot · 2022-06-02T09:54:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bowei, cezarygerard, kl52752

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~~ [bowei]

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

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 5, 2022

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 5, 2022

k8s-ci-robot requested review from rramkumar1 and thockin May 5, 2022 15:41

k8s-ci-robot assigned bowei May 5, 2022

kl52752 reviewed May 5, 2022

View reviewed changes

pkg/healthchecks/healthchecks_l4.go Outdated Show resolved Hide resolved

pkg/healthchecks/healthchecks_l4.go Outdated Show resolved Hide resolved

kl52752 reviewed May 5, 2022

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 5, 2022

bowei reviewed May 6, 2022

View reviewed changes

kl52752 reviewed May 6, 2022

View reviewed changes

cezarygerard force-pushed the awesome-firewall-healthchecks branch 5 times, most recently from 5720795 to 9e17971 Compare May 6, 2022 19:59

This was referenced May 6, 2022

Easier l4 hc - without singleton #1708

Closed

Easier l4 hc without singleton cezarygerard/ingress-gce#2

Closed

Easier l4 health checks - without singleton cezarygerard/ingress-gce#3

Open

kl52752 reviewed May 9, 2022

View reviewed changes

cezarygerard force-pushed the awesome-firewall-healthchecks branch from 570dc6e to a10c879 Compare May 9, 2022 23:01

bowei reviewed May 17, 2022

View reviewed changes

cezarygerard force-pushed the awesome-firewall-healthchecks branch 2 times, most recently from eccf309 to abf0c05 Compare May 17, 2022 17:58

bowei reviewed May 18, 2022

View reviewed changes

cezarygerard force-pushed the awesome-firewall-healthchecks branch from abf0c05 to d65a8ca Compare May 26, 2022 15:54

cezarygerard force-pushed the awesome-firewall-healthchecks branch from d65a8ca to 217b108 Compare May 27, 2022 10:54

panslava reviewed May 27, 2022

View reviewed changes

pkg/healthchecks/healthchecks_l4.go Outdated Show resolved Hide resolved

panslava reviewed May 27, 2022

View reviewed changes

pkg/healthchecks/healthchecks_l4.go Outdated Show resolved Hide resolved

panslava mentioned this pull request May 27, 2022

WIP: Implement migration from L4 Legacy Target Pool based services to L4 RBS #1721

Closed

cezarygerard force-pushed the awesome-firewall-healthchecks branch from 217b108 to 91c10b1 Compare May 30, 2022 14:59

Rewrite L4 healthchecks: Apply review comments

f8353dc

EnsureL4HealthCheck: replace lengthy return value list in with named struct Improve firewall rule comparison Added debug logs Improved go fmt Renamed function names And many more small ones

cezarygerard force-pushed the awesome-firewall-healthchecks branch from 91c10b1 to f8353dc Compare May 31, 2022 09:54

panslava reviewed Jun 1, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 1, 2022

k8s-ci-robot assigned kl52752 Jun 2, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 2, 2022

k8s-ci-robot merged commit 5382feb into kubernetes:master Jun 2, 2022

Rewrite L4 healthchecks creation and deletion #1705

Rewrite L4 healthchecks creation and deletion #1705

Conversation

cezarygerard commented May 5, 2022

k8s-ci-robot commented May 5, 2022

cezarygerard commented May 5, 2022

bowei commented May 5, 2022

bowei commented May 5, 2022

cezarygerard commented May 5, 2022

bowei left a 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

cezarygerard May 6, 2022 • edited Loading

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

bowei commented May 9, 2022

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

cezarygerard commented May 9, 2022

bowei May 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bowei left a 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

cezarygerard May 26, 2022 • edited Loading

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

cezarygerard commented May 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bowei May 18, 2022 • edited Loading

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

cezarygerard May 25, 2022 • edited Loading

cezarygerard May 6, 2022 •

edited

Loading

bowei May 17, 2022 •

edited

Loading

cezarygerard May 26, 2022 •

edited

Loading

bowei May 18, 2022 •

edited

Loading

cezarygerard May 25, 2022 •

edited

Loading

bowei May 18, 2022 •

edited

Loading

cezarygerard May 25, 2022 •

edited

Loading