Add disableStackPrefix option to disable using eksctl prefix in stack naming #3600

omnibrian · 2021-04-18T15:13:14Z

Description

Based on the discussion in #581, add a config option to disable using eksctl- prefix on all CloudFormation stacks created and managed by eksctl. Change stack naming of eksctl-{{ clusterName }}-.* to {{ clusterName }}-.* defaulting to including the stack prefix for backwards compatibility.

To address #2943, replace any underscores in the stack name with dashes to satisfy CloudFormation stack name validation allowing usage of cluster names that include underscores.

Example configuration to remove stack prefixing:

apiVersion: eksctl.io/v1alpha5
kind: ClusterConfig

metadata:
  name: testing_123
  region: us-east-1
  disableStackPrefix: true
  version: '1.19'

The above configuration gets the following stack names:

testing-123-cluster
testing-123-addon-iamserviceaccount-kube-system-aws-node
testing-123-nodegroup-ng-1

Checklist

Added tests that cover your change (if possible)
Added/modified documentation as required (such as the README.md, or the userdocs directory)
Manually tested
Made sure the title of the PR is a good description that can go into the release notes
(Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Backfilled missing tests for code in same general area 🎉
Refactored something and made the world a better place 🌟

Callisto13 · 2021-04-26T08:08:20Z

👍 thanks @omnibrian, the team will get to reviewing this soon

Callisto13 · 2021-04-26T08:19:13Z

pkg/ctl/delete/cluster.go

@@ -28,6 +28,7 @@ func deleteClusterCmd(cmd *cmdutils.Cmd) {
 	cmd.FlagSetGroup.InFlagSet("General", func(fs *pflag.FlagSet) {
 		fs.StringVarP(&cfg.Metadata.Name, "name", "n", "", "EKS cluster name")
 		cmdutils.AddRegionFlag(fs, &cmd.ProviderConfig)
+		cmdutils.AddDisableStackPrefixFlag(fs, &cfg.Metadata.DisableStackPrefix)


What would happen if a user forgets to give this flag here (or on any delete command)? I am not sure users would expect to need to provide flags/config like this for deletes; does it fail fast enough when it can't find the stack and is then retryable, or could people be left with resources lying around?

This seems to tie in with your other comment about only having this flag for creates and automatically discovering clusters/stacks with or without the stack prefix for all other commands. Thinking about it a bit, I really agree with making it an easier experience to use without having to constantly remember about including the cli flag. I'm going to dig in and see if I can figure out a way to handle that better.

If all commands supported using the config file, I would say this is the kind of thing to make config file only and not even include a flag and then any operations on. Do you know if #1126 is going to be completed in the near future? That could be another option to get around this.

Do you know if #1126 is going to be completed in the near future? That could be another option to get around this.

We are a little stretched atm, so this has fallen down our priority list. I feel that making it config only would just improve thie UX a little given the imperative nature of eksctl: people often will throw config together on the fly for a one-off command since each command only processes the conf it cares about.

Annoyingly I had a solution for this issue all mapped out (a little more convoluted, but good UX as you only set the name once) but like a total eejit I wrote it in the community slack and so now the history is gone 😫 🤦‍♀️ . Am trying to scrape it from my memory...

pkg/actions/fargate/tasks.go

Callisto13 · 2021-04-26T11:59:36Z

The team is slightly concerned about the UX for this. Ideally, a user would provide an option like this just once (eg on create cluster), with eksctl figuring out the rest for subsequent actions, especially deletes. Having it required for every subsequent command doesn't feel right. What do people think?

aclevername · 2021-04-28T14:41:08Z

The team is slightly concerned about the UX for this. Ideally, a user would provide an option like this just once (eg on create cluster), with eksctl figuring out the rest for subsequent actions, especially deletes. Having it required for every subsequent command doesn't feel right. What do people think?

thats my biggest concern before. I'm not sure what the best route/if its possible for us to get to this scenario. This is mainly because we rely heavily on the stack names because cloudformation doesn't let you search for stacks matching a tag!

Callisto13 · 2021-06-04T10:10:42Z

Hey @omnibrian! Just checking in on the status of this, do you plan to continue development or would you like it taken over? Thanks!

omnibrian · 2021-06-07T01:13:38Z

Hey @omnibrian! Just checking in on the status of this, do you plan to continue development or would you like it taken over? Thanks!

So sorry that this PR ended up getting a bit stale. I wouldn't mind continuing development and I'm hoping to have some free time coming up to work on this in the next few weeks but I understand if this is something you are wanting to get completed and merged on a sooner timeline.

Callisto13 · 2021-06-07T08:09:58Z

There's no rush @omnibrian, I was just going some housekeeping 😄 . It's awesome that you are up for continuing, would you mind converting to a draft or closing while you are working on it?

Callisto13 · 2021-06-07T08:23:23Z

I also managed to find the notes for one of the implementation ideas we had a while ago. It is up for debate, so if you think it has merit feel free to adapt and run with it. (I wrote this a while ago so some of the links and snippets may be dated)

The team has had some discussion around this and we have found an implementation which will give a decent UX for adding Cf stack prefixes without increasing API calls.
We can think about prefixes for other resources at a later date if they are still necessary.

Configuration:
- There will be a new object in the Config which will contain a single field: cloudFormation.stackNamePrefix
- This field is optional, and will only take effect if set during cluster create
On cluster creation:
- The prefix value will be added to the cluster stack name (somewhere around here)
- The prefix will be saved as a tag on the cluster: alpha.eksctl.io/stack-prefix, as part of an UpdateClusterTags task (eg). (we cannot do this during create as CF does not support this)
- Likewise, the full stack name will also be saved as a tag on the cluster, again after create: alpha.eksctl.io/cluster-stack-name
Current uses of makeClusterStackName (func) would be replaced or altered by looking up the cached tag
- The tags are already cached when we call ctl.CanOperate in most commands
- This implementation will therefore look for the stack-prefix tag value in that cache, and use it to generate the stack names
For creating nodegroups:
- makeNodeGroupStackName (here) will receive the prefix from the cached cluster tag value
- The resulting nodegroup stack name will be saved as a tag on the nodegroup: alpha.eksctl.io/nodegroup-stack-name

One important thing to consider is that AWS silently truncates stack names. This means that if a name is too long, certain identifying markers which eksctl needs to work may be lost. This could cause things to break in unexpected ways. We therefore probably need some documentation to warn users that prefixes (and indeed long cluster names) are used at their own risk.

omnibrian · 2021-06-07T19:26:32Z

@Callisto13 I just set the status to be a draft PR, thank you for those notes!

A lot of that makes sense and I'd like to try playing around with tagging as that would be a good solution to the deletes not needing to be aware of whether prefix was included or not. I just think to @aclevername's point earlier, the only way to properly use that would be to list all stacks and then filter the results based on tagging because of the inability to lookup stacks by tag yet. Would listing and then filtering by tag be acceptable? Or maybe doing something like creating a resource group to do the query and then listing the stacks in that resource group?

gwudakar · 2021-08-05T16:53:34Z

Is there a version that this stack prefix is implemented in?

Callisto13 · 2021-08-06T08:19:29Z

Not yet @gwudakar, I am not sure if @omnibrian is carrying on with this. If not, feel free to pick it up.

Brian: I realise you asked some questions in your last comment which I never answered 🤦‍♀️ I will remind myself of the context and answer soon

gwudakar · 2021-08-06T14:24:38Z

I thought @omnibrian 's PR was complete? It looks pretty good, but I don't have access to all the details. I did a minimal code review and it looks like it was implemented based on your comment from Jun 7.

Callisto13 · 2021-08-06T14:27:39Z

Huh I didn't see any new commits come in 🤔 I'll take a look

Callisto13 · 2021-08-06T14:31:04Z

Implementation is the same as it was initially, using the metadata.disableFlagPrefix option (required on every command) and dynamically generating the name with each call.

Callisto13 · 2021-08-25T11:24:52Z

I just think to @aclevername's point earlier, the only way to properly use that would be to list all stacks and then filter the results based on tagging because of the inability to lookup stacks by tag yet. Would listing and then filtering by tag be acceptable? Or maybe doing something like creating a resource group to do the query and then listing the stacks in that resource group?

Listing all stacks and then filtering may be quite expensive... but I feel we do things like this often simply because we don't have a choice.

The idea of adding a tag with the exact stack name to the cluster (we already get cluster details with every command anyway) is a potential hack to get around this.

Skarlso · 2021-11-16T08:46:52Z

Hi @omnibrian.

Are you planning on finishing this, or is can we go ahead and wrap it up internally?

Cheers!

bnannier · 2022-04-12T14:29:30Z

Why do you guys keep closing this pull request?

Skarlso · 2022-04-12T14:31:48Z

Why do you guys keep closing this pull request?

Hi. :)

What do you mean? It's not closed.

omnibrian · 2022-04-15T19:18:36Z

Very sorry for such a long radio silence, life got busy and I kind of forgot that I had left this open.

I just gave it a shot at reimplementing with the idea of storing the flag to disable stack prefixes in the tags on the control plane. I also updated the cloudformation stacks lookup to have the stack name regex be a bit more broad (ignore any kind of prefixing) and then filter for stacks by the cluster-name tag since that gets added to all stacks created by eksctl so hopefully that can be a happy medium for looking up stacks by tag. I'm still fairly new to golang and the eksctl codebase though, so let me know if this isn't really the right direction for how to implement all that and I can fix it.

Let me know what you all think 🙏

Skarlso · 2022-04-15T19:33:32Z

Thanks, @omnibrian! The team is on vacation atm so bear with us for a bit. :)

bnannier · 2022-04-26T00:01:54Z

@Himangini @cPu1 @Skarlso
Can on of you guys please review and approve?

Skarlso · 2022-04-26T07:19:06Z

@bnannier Hi. :)

We will do that eventually, but please bear in mind that our capacity is limited. :)

thanks

Skarlso · 2022-04-26T19:08:46Z

@omnibrian Hey! Do you have any kind of proximation of how much more API calls this will incur?

I started reading this PR, I don't know yet, but I assume every time we constructed a name or tried searching for a resource, you have to replace that with a name call. Or, alternatively, do it once at the begin and pass it along to every other call?

Huh, actually, considering that, this appears to be a lot smaller PR than I would imagine is needed.

omnibrian · 2022-04-26T23:01:22Z

Do you have any kind of proximation of how much more API calls this will incur?

Since I didn't add any calls to the AWS SDK directly, I'm hoping that there aren't too many more DescribeStack calls from relaxing the regex (considering most users wouldn't try using the same eksctl suffixes but with different prefixes). I'm trying to think if there's other places that could be incurring more API calls but I don't think there's any other spots that will be adding more API calls.

I started reading this PR, I don't know yet, but I assume every time we constructed a name or tried searching for a resource, you have to replace that with a name call. Or, alternatively, do it once at the begin and pass it along to every other call?

Huh, actually, considering that, this appears to be a lot smaller PR than I would imagine is needed.

I was kind of surprised at how small the changeset ended up being (that does make me worry that I might've missed some code paths though). The idea is to centralize naming into that ClusterConfig.MakeStackName() func instead of being spread throughout the different files since everywhere that was building up a stack name already has access to a ClusterConfig.

Skarlso · 2022-04-27T06:33:23Z

pkg/actions/fargate/tasks.go

 func (t *createFargateStackTask) Do(errs chan error) error {
 	rs := builder.NewFargateResourceSet(t.cfg)
 	if err := rs.AddAllResources(); err != nil {
 		return errors.Wrap(err, "couldn't add all resources to fargate resource set")
 	}
-	return t.stackManager.CreateStack(context.TODO(), makeClusterStackName(t.cfg.Metadata.Name), rs, nil, nil, errs)
+	return t.stackManager.CreateStack(context.TODO(), t.cfg.MakeStackName("fargate"), rs, nil, nil, errs)


Suggested change

return t.stackManager.CreateStack(context.TODO(), t.cfg.MakeStackName("fargate"), rs, nil, nil, errs)

"fargate"

This should probably be a constant somewhere that is accessible. Like in the api package.

If I'm moving this to a constant in api then I would rather be moving all constant stack suffixes into there, is there anything against just having the naming funcs be in that package too?

Skarlso · 2022-04-27T06:34:16Z

pkg/apis/eksctl.io/v1alpha5/assets/schema.json

@@ -466,6 +466,12 @@
          "x-intellij-html-description": "arbitrary metadata ignored by <code>eksctl</code>.",
          "default": "{}"
        },
+        "disableStackPrefix": {


So, this is only for stacks, right? Not other resources eksctl prefixes, such as roles, service accounts, etc...?

That makes sense now. I see why this pr is as small as this. :D

Yeah, I was under the impression that things like roles and service accounts could have their names overridden already too. Since the idea was more around allowing users to use their own naming conventions for cloudformation stacks specifically I honestly didn't really dig around much beyond that.

Well, yes and no. You can't override some of the roles and you can't override many of the internal resources like, subnet routing or whatever, which gets created. Mostly roles...

Ah cool, I can dig around to see what I can find that could benefit from inheriting the disabled prefix 👍

Skarlso · 2022-04-27T06:35:44Z

pkg/apis/eksctl.io/v1alpha5/types.go

+
+	for k, v := range cluster.Tags {
+		if k == DisableStackPrefixTag {
+			if disableStackPrefix, err := strconv.ParseBool(*v); err == nil {


We have a thing called, IsEnabled or something like that in the api pkg.

That only works for a bool pointer, in this case it's a string pointer that's getting parsed for a bool value. That said, I did only decide to use the string "true"/"false" so that it's easier for users to read it from the stack tags too. If we go with not having the disableStackPrefix tag at all on stacks where it's not true, then this could easily just be a check for the existence of that tags key.

Skarlso · 2022-04-27T06:36:29Z

pkg/apis/eksctl.io/v1alpha5/types.go

+// MakeStackName builds a consistent name for a CloudFormation stack
+func (c *ClusterConfig) MakeStackName(suffixes ...string) string {
+	return c.MakeStackNameFromName(c.Metadata.Name, suffixes...)
+}
+
+// MakeStackNameFromName builds a consistent name for a CloudFormation stack
+func (c *ClusterConfig) MakeStackNameFromName(clusterName string, suffixes ...string) string {
+	stackName := clusterName
+	if !c.Metadata.DisableStackPrefix {
+		stackName = fmt.Sprintf("eksctl-%s", stackName)
+	}
+	for _, suffix := range suffixes {
+		stackName = fmt.Sprintf("%s-%s", stackName, suffix)
+	}
+	return stackName
+}
+


Why are there two of these?

Because of this in manager pkg for cluster naming: https://github.com/weaveworks/eksctl/blob/main/pkg/cfn/manager/cluster.go#L29-L42

I figured there was a reason for that second method existing there so this just became a way to leave those method signatures intact. I can go through and find what's actually using that StackCollection.MakeStackNameFromName() though and reorganize the code back to using just MakeStackName() if you think that's a better course of action.

Skarlso · 2022-04-27T06:42:29Z

pkg/awsapi/ssm.go

@@ -11,29 +11,30 @@ import (
 // SSM provides an interface to the AWS SSM service.


Hm, this stems from the update, right?

Yeah, I couldn't figure out a way to make build without this getting changed, do you know if there's some env vars or a flag I should be using during make build to avoid these overwrites?

Sadly, I don't. These can be annoying, indeed.

Skarlso · 2022-04-27T06:43:14Z

pkg/cfn/builder/cluster.go

@@ -320,7 +320,12 @@ func (c *ClusterResourceSet) addResourcesForControlPlane(subnetDetails *SubnetDe
 }

 func makeCFNTags(clusterConfig *api.ClusterConfig) []gfncfn.Tag {
-	var tags []gfncfn.Tag
+	tags := []gfncfn.Tag{


Should this tag be only created when it's enabled?

That's definitely an option here since we have to work with it not being there on stacks that would've been created before this code is in place, are you leaning more towards only tagging stacks where the prefix is disabled?

Yeah I think we should go with that route. I think it makes it easy then to parse the value because it's enough to check if it exists or not, right?

Skarlso · 2022-04-27T06:46:44Z

pkg/cfn/manager/api.go

@@ -32,8 +32,8 @@ const (
 	resourceTypeAutoScalingGroup = "auto-scaling-group"
 	outputsRootPath              = "Outputs"
 	mappingsRootPath             = "Mappings"
-	ourStackRegexFmt             = "^(eksctl|EKS)-%s-((cluster|nodegroup-.+|addon-.+|fargate|karpenter)|(VPC|ServiceRole|ControlPlane|DefaultNodeGroup))$"
-	clusterStackRegex            = "eksctl-.*-cluster"
+	ourStackRegexFmt             = "^.*%s-((cluster|nodegroup-.+|addon-.+|fargate|karpenter)|(VPC|ServiceRole|ControlPlane|DefaultNodeGroup))$"


I wonder what problems this could cause with existing stacks created by other users or instances.

The .*-cluster for example will match everything that is not eksctl but has -cluster in it. Including everything anyone created outside of eksctl.

Also, shouldn't this regex be dynamically created? For example, if disable is enabled, it's more relaxed, but otherwise, it's still eksctl-.*-cluster?

Originally having two separate regexes based on the disable flag was the implementation but this goes back to @Callisto13's comment about UX and not wanting users to have to know whether the stack prefixing was disabled for every subsequent command past the create. If we don't want to force the user to let us know on a get/update/delete/etc if the stack prefixing was disabled, we need to check for both to cover each case every time.

As for matching on -cluster, that would still include the cluster name at that %s so it would limit to .*-myclustername-cluster which is much more narrow than .*-cluster thankfully. That said, I can tighten up the regex so it's only a triple of (eksctl-|EKS-|) as the prefixes to DescribeStack on, what do you think?

I just realized that comment was about the line below (clusterStackRegex) instead of ourStackRegexFmt. That regex is only used for listing all clusters (eksctl get cluster without --name) so I feel like the UX for that should be that all clusters get returned regardless of prefixing instead of only having a list of un-prefixed clusters or only prefixed clusters. If there's a better way to reduce the false positives that .*-cluster would return without accidentally inducing false negatives I'm up for any ideas you might have!

Skarlso · 2022-04-27T06:57:53Z

pkg/cfn/manager/api.go

@@ -463,7 +465,19 @@ func (c *StackCollection) ListStacksMatching(ctx context.Context, nameRegex stri
 					// this shouldn't return the error, but just stop the pagination and return whatever it gathered so far.
 					return stacks, nil
 				}
-				stacks = append(stacks, stack)
+


I'm not sure I follow this one. Maybe I'm just not reading this right. But before we always added the stack. And we keep always adding the stack regardless of the outcome of the search. I see that you wanted to avoid adding the stack twice, but that's an indicator of the fact that it's always being added if MatchString is true, right?

You add it if the tag is there otherwise... we add it. :D Do I read that right? :D

Sort of, in an effort to keep the flow from the old where it wasn't inspecting tags, I'm making the assumption that a stack without a alpha.eksctl.io/cluster-name tag should still get added. But if that tag is there, it only gets included if the value matches the known cluster name that the user passed in. The pseudo-code is basically this:

if (stack.ClusterNameTagValue != clusterName) { skip } else { stack = append(stacks, stack) }

This would work better if we can completely ignore stacks without that cluster name tag but I'm fairly sure that'll break backwards compatibility though.

Want me to add a comment in the code describing the flow?

Oh yeah you're right. It only adds it if the name equals. Nah, no need for the comment but maybe make it more obvious by using a predicate functions which is a closure and returns true or false. That's a lot more readable and doesn't require the extra bool value and has the added benefit of stopping as soon as the tag is found.

Something like this, but with different names maybe :)

hasClusterNameTag := func() (bool, bool) { for _, t := range stack.Tags { if *t.Key == api.ClusterNameTag && c.spec.Metadata.Name != "" { if *t.Value == c.spec.Metadata.Name { return true, true } return true, false } } return false, false } if has, match := hasClusterNameTag(); has && match { stacks = append(stacks, stack) } else if !has { stacks = append(stacks, stack) }

In trying to write a getClusterName I found that there's already one available that works perfectly for this use case so I just leaned on that for this logic, let me know what you think.

If you found something then use that. :D 👍 👍

pkg/ctl/cmdutils/filter/nodegroup_filter_test.go

Skarlso · 2022-04-27T06:59:45Z

pkg/cfn/manager/iam.go

@@ -15,7 +14,7 @@ import (

 // makeIAMServiceAccountStackName generates the name of the iamserviceaccount stack identified by its name, isolated by the cluster this StackCollection operates on and 'addon' suffix
 func (c *StackCollection) makeIAMServiceAccountStackName(namespace, name string) string {
-	return fmt.Sprintf("eksctl-%s-addon-iamserviceaccount-%s-%s", c.spec.Metadata.Name, namespace, name)
+	return c.spec.MakeStackName("addon", "iamserviceaccount", namespace, name)


I think we do something in Kuberentes part as well, right? Reference this name somewhere in a config map I think?

Are you sure it uses the stack name? I was fairly sure the serviceaccount object pointed at the role name instead. I'll go through and check the code / test in a cluster to see what it does to make sure it's not broken.

Nope, not certain. :D Thanks. :)

So it is only the role name and not the stack name that get used in the service account annotation, everything seems to be working fine from my dev testing too

Awesome! Thank you for confirming.

Skarlso

Lots of questions really for now. Also, we definitely should create an integration test for this.

nethershaw · 2024-07-31T07:10:13Z

After all of that work... you just forget it?

Callisto13 mentioned this pull request Apr 19, 2021

Adding an option to choose the stack name that will be created when we run eksctl create call. #3599

Closed

omnibrian changed the title ~~Add stack prefix option for changing eksctl stack naming~~ Add disableStackPrefix option to disable using eksctl prefix in stack naming Apr 24, 2021

Callisto13 added the kind/feature New feature or request label Apr 26, 2021

Callisto13 reviewed Apr 26, 2021

View reviewed changes

pkg/actions/fargate/tasks.go Outdated Show resolved Hide resolved

Callisto13 removed a link to an issue Jun 7, 2021

Cloudformation stack validation error because of underscore in EKS cluster name #2943

Closed

omnibrian marked this pull request as draft June 7, 2021 19:10

Skarlso mentioned this pull request Jan 19, 2022

Cloudformation stack validation error because of underscore in EKS cluster name #2943

Closed

omnibrian marked this pull request as ready for review April 15, 2022 19:19

Skarlso requested review from cPu1, Skarlso and Himangini April 15, 2022 19:32

Skarlso reviewed Apr 27, 2022

View reviewed changes

pkg/ctl/cmdutils/filter/nodegroup_filter_test.go Show resolved Hide resolved

Skarlso reviewed Apr 27, 2022

View reviewed changes

omnibrian and others added 3 commits May 10, 2022 22:08

Allow disabling of eksctl stack prefix

9a34708

Merge branch 'main' into stack-prefix

a4ee549

Merge branch 'main' into stack-prefix

d07b4c6

Himangini removed their request for review May 26, 2022 10:53

Himangini added the stale label May 31, 2022

github-actions bot closed this Jun 6, 2022

nethershaw mentioned this pull request Jul 31, 2024

Option to pass stack names to the CLI as flags/parameters #581

Closed

	return t.stackManager.CreateStack(context.TODO(), t.cfg.MakeStackName("fargate"), rs, nil, nil, errs)
	"fargate"

		@@ -11,29 +11,30 @@ import (
		// SSM provides an interface to the AWS SSM service.

Add disableStackPrefix option to disable using eksctl prefix in stack naming #3600

Add disableStackPrefix option to disable using eksctl prefix in stack naming #3600

Uh oh!

Conversation

omnibrian commented Apr 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

Uh oh!

Callisto13 commented Apr 26, 2021

Uh oh!

Callisto13 Apr 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Callisto13 commented Apr 26, 2021

Uh oh!

aclevername commented Apr 28, 2021

Uh oh!

Callisto13 commented Jun 4, 2021

Uh oh!

omnibrian commented Jun 7, 2021

Uh oh!

Callisto13 commented Jun 7, 2021

Uh oh!

Callisto13 commented Jun 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omnibrian commented Jun 7, 2021

Uh oh!

gwudakar commented Aug 5, 2021

Uh oh!

Callisto13 commented Aug 6, 2021

Uh oh!

gwudakar commented Aug 6, 2021

Uh oh!

Callisto13 commented Aug 6, 2021

Uh oh!

Callisto13 commented Aug 6, 2021

Uh oh!

Callisto13 commented Aug 25, 2021

Uh oh!

Skarlso commented Nov 16, 2021

Uh oh!

bnannier commented Apr 12, 2022

Uh oh!

Skarlso commented Apr 12, 2022

Uh oh!

omnibrian commented Apr 15, 2022

Uh oh!

Skarlso commented Apr 15, 2022

Uh oh!

bnannier commented Apr 26, 2022

Uh oh!

Skarlso commented Apr 26, 2022

Uh oh!

Skarlso commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omnibrian commented Apr 26, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

omnibrian commented Apr 18, 2021 •

edited

Loading

Callisto13 Apr 26, 2021 •

edited

Loading

Callisto13 commented Jun 7, 2021 •

edited

Loading

Skarlso commented Apr 26, 2022 •

edited

Loading