ceph: remove RADOS options from CephNFS and use .nfs pool #8501

josephsawaya · 2021-08-05T17:28:44Z

This commit updates the CephNFS CR to make the RADOS settings optional
for Ceph versions above 16.2.6 due to the NFS module changes in Ceph.
The changes in Ceph make it so the RADOS pool is always ".nfs" and the
RADOS namespace is always the name of the NFS cluster.

Description of your changes:

Which issue is resolved by this Pull Request:
Resolves #8450

Checklist:

travisn

What's the upgrade story? For example, if someone has nfs configured and they upgrade, will their existing shares stop working?
In what Ceph version is the .nfs pool required?

josephsawaya · 2021-08-06T04:16:36Z

Looks like this should help answer your questions.

What's the upgrade story? For example, if someone has nfs configured and they upgrade, will their existing shares stop working?

I'm not too familiar with how Rook handles upgrading but it looks like the NFS module should handle it on the backend, not too sure what would happen to the NFS CRDs though since there would be fields that aren't there anymore.

In what Ceph version is the .nfs pool required?

I'm also not too familiar with Ceph versioning but it looks like it would be required in later versions of Pacific and onward?

leseb

It looks like this is for Quincy only and won't be backported to Pacific. But sill we need a good upgrade story for this. Perhaps we can just rename whatever pool was used for NFS data with .nfs. That should be sufficient IMO to not break existing clusters. However, this needs to be carefully tested.

leseb · 2021-08-06T07:58:58Z

pkg/operator/ceph/nfs/nfs.go

@@ -254,6 +255,21 @@ func instanceName(n *cephv1.CephNFS, name string) string {
 	return fmt.Sprintf("%s-%s-%s", AppName, n.Name, name)
 }

+// create and enable .nfs RADOS pool
+func createNFSRADOSPool(context *clusterd.Context, clusterInfo *cephclient.ClusterInfo) error {
+	args := []string{"osd", "pool", "create", ".nfs"}


Suggested change

args := []string{"osd", "pool", "create", ".nfs"}

args := []string{"osd", "pool", "create", radosPoolName}

leseb · 2021-08-06T07:59:02Z

pkg/operator/ceph/nfs/nfs.go

+	if err != nil {
+		return err
+	}
+	args = []string{"osd", "pool", "application", "enable", ".nfs", "nfs"}


leseb · 2021-08-06T07:59:47Z

pkg/operator/ceph/nfs/nfs.go

+		// Create the .nfs pool if it doesn't already exist
+		err := createNFSRADOSPool(context, clusterInfo)
+		if err != nil {
+			return errors.Wrapf(err, ".nfs pool not found and unable to create it ")


Suggested change

return errors.Wrapf(err, ".nfs pool not found and unable to create it ")

return errors.Wrapf(err, "failed to find %q pool and unable to create it", radosPoolName)

josephsawaya · 2021-08-06T13:16:14Z

It looks like this is for Quincy only and won't be backported to Pacific. But sill we need a good upgrade story for this. Perhaps we can just rename whatever pool was used for NFS data with .nfs. That should be sufficient IMO to not break existing clusters. However, this needs to be carefully tested.

I think it's possible that before, users were able to use a different pool for each NFS daemon, there might've been multiple pools containing NFS data, so I don't think renaming the pool would work in that case.

leseb · 2021-08-06T13:47:49Z

It looks like this is for Quincy only and won't be backported to Pacific. But sill we need a good upgrade story for this. Perhaps we can just rename whatever pool was used for NFS data with .nfs. That should be sufficient IMO to not break existing clusters. However, this needs to be carefully tested.

I think it's possible that before, users were able to use a different pool for each NFS daemon, there might've been multiple pools containing NFS data, so I don't think renaming the pool would work in that case.

🤔 Hum yes, if you had multiple CephNFS servers running? AFAIK Ganesha HA support hasn't been that good. I wonder how common this is. Still, we should document a migration path we believe that would work some scenarios.

travisn · 2021-08-06T17:05:05Z

As discussed in huddle, after we confirm which version of Ceph has the changes we can add a check in Rook for the version where the new approach applies. Search for cephVersion.IsAtLeast and you'll see a few examples of checking the ceph version.

josephsawaya · 2021-08-06T17:21:48Z

As discussed in huddle, after we confirm which version of Ceph has the changes we can add a check in Rook for the version where the new approach applies. Search for cephVersion.IsAtLeast and you'll see a few examples of checking the ceph version.

So the CephNFS CRD would have to support both approaches?

travisn · 2021-08-06T18:03:49Z

As discussed in huddle, after we confirm which version of Ceph has the changes we can add a check in Rook for the version where the new approach applies. Search for cephVersion.IsAtLeast and you'll see a few examples of checking the ceph version.

So the CephNFS CRD would have to support both approaches?

Yes, Rook really needs to deal with the versioning issues since Rook is supporting multiple versions of Ceph. NFS is not used as commonly so I think we realistically don't need to maintain the full compatibility, but we need to at least have some time period where both modes are supported and a migration plan is fully understood.

josephsawaya · 2021-08-06T18:19:25Z

As discussed in huddle, after we confirm which version of Ceph has the changes we can add a check in Rook for the version where the new approach applies. Search for cephVersion.IsAtLeast and you'll see a few examples of checking the ceph version.

So the CephNFS CRD would have to support both approaches?

Yes, Rook really needs to deal with the versioning issues since Rook is supporting multiple versions of Ceph. NFS is not used as commonly so I think we realistically don't need to maintain the full compatibility, but we need to at least have some time period where both modes are supported and a migration plan is fully understood.

Ok so I guess an approach could be to keep the CRDs the same as they are right now and just make it clear through the documentation that for Ceph versions newer than Pacific 16.2.5 (or whatever version the NFS changes will be implemented in) those fields in the CRD aren't actually used and Ceph uses the default ".nfs" pool.

For the unit tests and integration tests, how would I write tests for different Ceph versions?

travisn · 2021-08-06T19:12:43Z

Yes, Rook really needs to deal with the versioning issues since Rook is supporting multiple versions of Ceph. NFS is not used as commonly so I think we realistically don't need to maintain the full compatibility, but we need to at least have some time period where both modes are supported and a migration plan is fully understood.

Ok so I guess an approach could be to keep the CRDs the same as they are right now and just make it clear through the documentation that for Ceph versions newer than Pacific 16.2.5 (or whatever version the NFS changes will be implemented in) those fields in the CRD aren't actually used and Ceph uses the default ".nfs" pool.

Yes, if we leave the settings in the CRD, then document the difference, that should work. In newer Ceph versions, those properties would just be ignored.

For the unit tests and integration tests, how would I write tests for different Ceph versions?

We run the integration tests on multiple versions of Kubernetes already, so we should already have coverage in those tests across versions. For the unit tests, you could just test smaller functions that wouldn't worry about setting different ceph versions.

travisn

LGTM just one final question

pkg/operator/ceph/nfs/nfs.go

travisn

LGTM. To test the upgrades manually, I'd suggest you deploy nfs from the rook master and ceph/ceph:v16.2.5, and verify it's working based on the old approach. Then you can update the ceph image in the cephcluster CR to latest pacific (quay.io/ceph/daemon-base:latest-pacific-devel) and confirm the nfs upgrade works.

feedback addressed

josephsawaya · 2021-08-17T13:39:10Z

LGTM. To test the upgrades manually, I'd suggest you deploy nfs from the rook master and ceph/ceph:v16.2.5, and verify it's working based on the old approach. Then you can update the ceph image in the cephcluster CR to latest pacific (quay.io/ceph/daemon-base:latest-pacific-devel) and confirm the nfs upgrade works.

There's more issues with this, I tried running a cluster on Ceph 16.2.5 and it looks like that version of Ceph (and some preceding versions) also has a default pool name so it's impossible to create exports, not sure if the diff between 16.2.5 and the master branch is just the name of the default pool or if anything else in the NFS module changed.

I somewhat fixed the issue mentioned above, and the exports aren't moved to the correct pool when you upgrade the Ceph version:

[root@rook-ceph-tools-78cdfd976c-f2shf /]# rados ls -p nfs-ganesha --all
my-nfs  conf-nfs.my-nfs
my-nfs  export-1
my-nfs  grace
my-nfs  rec-0000000000000002:my-nfs.a

[root@rook-ceph-tools-78cdfd976c-f2shf /]# rados ls -p .nfs --all
my-nfs  conf-nfs.my-nfs
my-nfs  grace
my-nfs  rec-0000000000000002:my-nfs.a

travisn · 2021-08-17T18:59:28Z

LGTM. To test the upgrades manually, I'd suggest you deploy nfs from the rook master and ceph/ceph:v16.2.5, and verify it's working based on the old approach. Then you can update the ceph image in the cephcluster CR to latest pacific (quay.io/ceph/daemon-base:latest-pacific-devel) and confirm the nfs upgrade works.

There's more issues with this, I tried running a cluster on Ceph 16.2.5 and it looks like that version of Ceph (and some preceding versions) also has a default pool name so it's impossible to create exports, not sure if the diff between 16.2.5 and the master branch is just the name of the default pool or if anything else in the NFS module changed.

I somewhat fixed the issue mentioned above, and the exports aren't moved to the correct pool when you upgrade the Ceph version:
[root@rook-ceph-tools-78cdfd976c-f2shf /]# rados ls -p nfs-ganesha --all
my-nfs  conf-nfs.my-nfs
my-nfs  export-1
my-nfs  grace
my-nfs  rec-0000000000000002:my-nfs.a

[root@rook-ceph-tools-78cdfd976c-f2shf /]# rados ls -p .nfs --all
my-nfs  conf-nfs.my-nfs
my-nfs  grace
my-nfs  rec-0000000000000002:my-nfs.a

@batrick @liewegas To migrate to the new .nfs pool in latest Pacific, any suggestions? Not sure many users will be impacted, but at least some manual procedure to update would be helpful.

travisn · 2021-08-19T19:16:00Z

pkg/operator/ceph/nfs/controller.go

+		if r.clusterInfo.CephVersion.IsAtLeast(version.CephVersion{Major: 16, Minor: 2, Extra: 6}) {
+			cephNFS.Spec.RADOS.Pool = dotNFSPoolName
+		} else {
+			cephNFS.Spec.RADOS.Pool = nfsGaneshaPoolName


Prior to 16.2.6 didn't the user have to set this in the CR, so we shouldn't overwrite it?

From what I can tell looking at the Ceph code,prior to 16.2.5 there was also a default pool name and the name was "nfs-ganesha" set here and used here, so it looks like Octopus is the version that allows customizing the pool name and namespace.

Your PR currently doesn't allow it to be set for octopus either. Seems like we can remove the else block to keep the current behavior that they set the pool name in the CR if older than 16.2.6

So the way I see it:

Octopus: Customization is allowed, so don't change the pool and namespace

Pacific before 16.2.6: No customization, default pool name is nfs-ganesha

Pacific after 16.2.6: No customization, default pool name is .nfs

This code is supposed to change the pool and namespace if the version is pacific, if its above 16.2.6 it overwrites it to ".nfs", else it changes it to "nfs-ganesha". If the version precedes Pacific it doesn't change it at all and it should be whatever the user provided in the spec.

Ok I see now that the pre-pacific settings won't be overwritten. Thanks for the clarification.

I think @josephsawaya last comment is valuable and should be a comment before this conditional block.

pkg/operator/ceph/nfs/nfs.go

travisn

The code changes look good. There is just an open question about what it will take to migrate existing users so we can add documentation.

travisn · 2021-08-20T16:31:25Z

pkg/operator/ceph/nfs/controller.go

+		if r.clusterInfo.CephVersion.IsAtLeast(version.CephVersion{Major: 16, Minor: 2, Extra: 6}) {
+			cephNFS.Spec.RADOS.Pool = dotNFSPoolName
+		} else {
+			cephNFS.Spec.RADOS.Pool = nfsGaneshaPoolName


Ok I see now that the pre-pacific settings won't be overwritten. Thanks for the clarification.

pkg/apis/ceph.rook.io/v1/types.go

leseb · 2021-08-23T14:30:36Z

Documentation/ceph-nfs-crd.md

@@ -91,6 +93,8 @@ ceph dashboard set-ganesha-clusters-rados-pool-namespace <cluster_id>:<pool_name
 * `pool`: The pool where ganesha recovery backend and supplemental configuration objects will be stored
 * `namespace`: The namespace in `pool` where ganesha recovery backend and supplemental configuration objects will be stored

+> **NOTE**: The RADOS settings aren't used in Ceph versions equal to or greater than Pacific 16.2.6, default values are used instead ".nfs" for the RADOS pool and the CephNFS CR's name for the RADOS namespace. However, RADOS settings are mandatory for versions preceding Pacific 16.2.6.


This should be part of the release note too. Probably in the breaking change section.

Is there something I can do about this on my end?

If it were in a minor release (v1.8), we would add it to the PendingReleaseNotes.md, but this will need to be in a patch release so I'll make a mental note for a special release note. Do we have an idea how soon 16.2.6 will be out? It would be good to include this in the rook release immediately before that ceph release.

Why not just waiting for v1.8?

Rook will not be usable with NFS in v16.2.6 or newer until we merge this. Since v16.2.6 is about to be released, we should get this in the next v1.7.x patch.

leseb · 2021-08-23T14:33:02Z

pkg/operator/ceph/nfs/controller.go

@@ -204,6 +205,15 @@ func (r *ReconcileCephNFS) reconcile(request reconcile.Request) (reconcile.Resul
 	}
 	r.clusterInfo.CephVersion = currentCephVersion

+	if r.clusterInfo.CephVersion.IsAtLeastPacific() {
+		if r.clusterInfo.CephVersion.IsAtLeast(version.CephVersion{Major: 16, Minor: 2, Extra: 6}) {


Please add version.CephVersion{Major: 16, Minor: 2, Extra: 6} as a variable and comment it to explain its purpose.

leseb · 2021-08-23T14:35:58Z

pkg/operator/ceph/nfs/controller.go

+		if r.clusterInfo.CephVersion.IsAtLeast(version.CephVersion{Major: 16, Minor: 2, Extra: 6}) {
+			cephNFS.Spec.RADOS.Pool = dotNFSPoolName
+		} else {
+			cephNFS.Spec.RADOS.Pool = nfsGaneshaPoolName


I think @josephsawaya last comment is valuable and should be a comment before this conditional block.

leseb · 2021-08-23T14:37:05Z

pkg/operator/ceph/nfs/nfs.go

@@ -37,6 +38,8 @@ import (

 const (
 	ganeshaRadosGraceCmd = "ganesha-rados-grace"
+	dotNFSPoolName       = ".nfs"


We need a more meaningful name for this. Like defaultNFSPoolName? Also add a comment this is hardcoded in Ceph.

leseb · 2021-08-23T14:37:12Z

pkg/operator/ceph/nfs/nfs.go

@@ -37,6 +38,8 @@ import (

 const (
 	ganeshaRadosGraceCmd = "ganesha-rados-grace"
+	dotNFSPoolName       = ".nfs"
+	nfsGaneshaPoolName   = "nfs-ganesha"


ditto, different name?

liewegas · 2021-09-02T20:05:14Z

The manual process for moving cluster mycluster exports from pool foo to .nfs is something like this:

ceph nfs cluster create mycluster 
for e in `rados -p foo ls -N mycluster | grep export`
do
  rados -p foo -N mycluster $e | ceph nfs export apply mycluster -i -
done

travisn · 2021-09-03T15:27:18Z

Sounds good to go ahead with this since we have the migration steps. Any word on how soon the 16.2.6 release will be? It would be ideal to have the next rook release with this change close to the same time.

leseb · 2021-09-09T13:53:47Z

Sounds good to go ahead with this since we have the migration steps. Any word on how soon the 16.2.6 release will be? It would be ideal to have the next rook release with this change close to the same time.

The pacific branch is blocked and QE has started yesterday. So if we don't backport this, we can go ahead, and once v1.8 is out v16.2.6 would have around for some time.

sebastian-philipp · 2021-09-09T14:44:29Z

does this depend on ceph/ceph#43075 ?

josephsawaya · 2021-09-09T15:42:26Z

does this depend on ceph/ceph#43075 ?

Yes

travisn

does this depend on ceph/ceph#43075 ?

Yes

@sebastian-philipp That is planned to be merged now for v16.2.6?

travisn · 2021-09-09T18:17:28Z

Documentation/ceph-nfs-crd.md

@@ -91,6 +93,8 @@ ceph dashboard set-ganesha-clusters-rados-pool-namespace <cluster_id>:<pool_name
 * `pool`: The pool where ganesha recovery backend and supplemental configuration objects will be stored
 * `namespace`: The namespace in `pool` where ganesha recovery backend and supplemental configuration objects will be stored

+> **NOTE**: The RADOS settings aren't used in Ceph versions equal to or greater than Pacific 16.2.6, default values are used instead ".nfs" for the RADOS pool and the CephNFS CR's name for the RADOS namespace. However, RADOS settings are mandatory for versions preceding Pacific 16.2.6.


Rook will not be usable with NFS in v16.2.6 or newer until we merge this. Since v16.2.6 is about to be released, we should get this in the next v1.7.x patch.

jmolmo

I have tested this and it works properly in the Rook part.

travisn

Per discussion, we will go ahead and merge this based on the 16.2.7 code check. The new code won't be triggered until that release though.

travisn · 2021-09-14T18:36:15Z

Documentation/ceph-nfs-crd.md

@@ -25,6 +25,8 @@ metadata:
  name: my-nfs
  namespace: rook-ceph
 spec:
+  # rados property is not used in versions of Ceph equal to or greater than 
+  # 16.2.6, see note in RADOS settings section below.


There are a few places like this in the docs/comments that still need to be changed from .6 to .7

travisn · 2021-09-14T20:41:34Z

pkg/operator/ceph/nfs/nfs.go

+				err := createDefaultNFSRADOSPool(context, clusterInfo, n.Spec.RADOS.Pool)
+				if err != nil {
+					return errors.Wrapf(err, "failed to find %q pool and unable to create it", n.Spec.RADOS.Pool)
+				}


We need to return nil here, right? Or else the reconcile will fail a few lines below when it returns pool not found

Suggested change

}

}

return nil

pkg/operator/ceph/nfs/nfs.go

travisn

Thanks for the tests!

mergify · 2021-09-17T17:41:24Z

This pull request has merge conflicts that must be resolved before it can be merged. @josephsawaya please rebase it. https://rook.io/docs/rook/master/development-flow.html#updating-your-fork

travisn · 2021-10-04T17:38:36Z

@mergify rebase

This commit updates the CephNFS CR to make the RADOS settings optional for Ceph versions above 16.2.7 due to the NFS module changes in Ceph. The changes in Ceph make it so the RADOS pool is always ".nfs" and the RADOS namespace is always the name of the NFS cluster. This commit also handles the changes in Ceph Pacific versions before 16.2.7 where the default pool name is "nfs-ganesha" instead of ".nfs". Closes: rook#8450 Signed-off-by: Joseph Sawaya <jsawaya@redhat.com>

mergify · 2021-10-04T17:39:49Z

Command rebase: success

Branch has been successfully rebased

Hey, I reacted but my real name is @Mergifyio

…port ceph: remove RADOS options from CephNFS and use .nfs pool (backport #8501)

ceph: remove RADOS options from CephNFS and use .nfs pool (backport #8501)

mergify bot added the ceph main ceph tag label Aug 5, 2021

travisn reviewed Aug 5, 2021

View reviewed changes

leseb previously requested changes Aug 6, 2021

View reviewed changes

josephsawaya force-pushed the ceph-nfs-remove-rados branch from 4335b6e to c66cbd4 Compare August 6, 2021 13:19

josephsawaya force-pushed the ceph-nfs-remove-rados branch 2 times, most recently from 6a23c94 to b6ce581 Compare August 9, 2021 20:07

josephsawaya requested a review from travisn August 10, 2021 19:10

travisn reviewed Aug 10, 2021

View reviewed changes

pkg/operator/ceph/nfs/nfs.go Show resolved Hide resolved

josephsawaya force-pushed the ceph-nfs-remove-rados branch 3 times, most recently from de4c6b1 to 2917e08 Compare August 11, 2021 16:30

travisn reviewed Aug 11, 2021

View reviewed changes

josephsawaya force-pushed the ceph-nfs-remove-rados branch from 2917e08 to e8801ea Compare August 19, 2021 19:09

travisn reviewed Aug 19, 2021

View reviewed changes

travisn reviewed Aug 20, 2021

View reviewed changes

leseb requested changes Aug 23, 2021

View reviewed changes

josephsawaya force-pushed the ceph-nfs-remove-rados branch from e8801ea to 96d601b Compare August 23, 2021 17:18

josephsawaya requested a review from leseb August 24, 2021 16:12

josephsawaya mentioned this pull request Sep 3, 2021

mgr/rook: update ceph orch apply nfs ceph/ceph#43046

Merged

3 tasks

travisn reviewed Sep 9, 2021

View reviewed changes

jmolmo approved these changes Sep 14, 2021

View reviewed changes

josephsawaya force-pushed the ceph-nfs-remove-rados branch from 96d601b to 31e4291 Compare September 14, 2021 15:32

travisn requested changes Sep 14, 2021

View reviewed changes

josephsawaya force-pushed the ceph-nfs-remove-rados branch from 31e4291 to 4cdb375 Compare September 14, 2021 18:53

travisn requested changes Sep 14, 2021

View reviewed changes

josephsawaya force-pushed the ceph-nfs-remove-rados branch from 4cdb375 to 4ef7342 Compare September 15, 2021 23:02

travisn approved these changes Sep 16, 2021

View reviewed changes

josephsawaya force-pushed the ceph-nfs-remove-rados branch from 4ef7342 to 828f440 Compare September 17, 2021 18:03

travisn requested review from leseb and removed request for leseb October 4, 2021 17:38

leseb force-pushed the ceph-nfs-remove-rados branch from 828f440 to ee791b0 Compare October 4, 2021 17:39

leseb approved these changes Oct 13, 2021

View reviewed changes

leseb merged commit e33d905 into rook:master Oct 13, 2021

travisn added the backport-release-1.7 label Oct 13, 2021

mergify bot mentioned this pull request Oct 13, 2021

ceph: remove RADOS options from CephNFS and use .nfs pool (backport #8501) #8969

Merged

josephsawaya mentioned this pull request Oct 13, 2021

ceph: remove RADOS options from CephNFS and use .nfs pool (backport #8501) #8970

Merged

10 tasks

leseb added a commit that referenced this pull request Oct 13, 2021

Merge pull request #8970 from josephsawaya/ceph-nfs-remove-rados-back…

f16266a

…port ceph: remove RADOS options from CephNFS and use .nfs pool (backport #8501)

travisn added a commit that referenced this pull request Oct 18, 2021

Merge pull request #8969 from rook/mergify/bp/release-1.7/pr-8501

4c60d0b

ceph: remove RADOS options from CephNFS and use .nfs pool (backport #8501)

BlaineEXE mentioned this pull request Oct 25, 2021

Enable nfs mgr module configuration of CephNFS #9021

Closed

3 tasks

	args := []string{"osd", "pool", "create", ".nfs"}
	args := []string{"osd", "pool", "create", radosPoolName}

	return errors.Wrapf(err, ".nfs pool not found and unable to create it ")
	return errors.Wrapf(err, "failed to find %q pool and unable to create it", radosPoolName)

ceph: remove RADOS options from CephNFS and use .nfs pool #8501

ceph: remove RADOS options from CephNFS and use .nfs pool #8501

Conversation

josephsawaya commented Aug 5, 2021 • edited

travisn left a comment

Choose a reason for hiding this comment

josephsawaya commented Aug 6, 2021

leseb 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

josephsawaya commented Aug 6, 2021

leseb commented Aug 6, 2021

travisn commented Aug 6, 2021

josephsawaya commented Aug 6, 2021

travisn commented Aug 6, 2021

josephsawaya commented Aug 6, 2021 • edited

travisn commented Aug 6, 2021

travisn left a comment

Choose a reason for hiding this comment

travisn left a comment

Choose a reason for hiding this comment

josephsawaya commented Aug 17, 2021 • edited

travisn commented Aug 17, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephsawaya Aug 20, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travisn 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

liewegas commented Sep 2, 2021

travisn commented Sep 3, 2021

leseb commented Sep 9, 2021 • edited

sebastian-philipp commented Sep 9, 2021

josephsawaya commented Sep 9, 2021

travisn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmolmo left a comment

Choose a reason for hiding this comment

travisn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

travisn left a comment

Choose a reason for hiding this comment

mergify bot commented Sep 17, 2021

travisn commented Oct 4, 2021

mergify bot commented Oct 4, 2021

josephsawaya commented Aug 5, 2021 •

edited

josephsawaya commented Aug 6, 2021 •

edited

josephsawaya commented Aug 17, 2021 •

edited

josephsawaya Aug 20, 2021 •

edited

leseb commented Sep 9, 2021 •

edited