Enhancements/oc/mirroring-manifests: Propose a new enhancement #238

jottofar · 2020-03-04T22:07:23Z

NOTE: Since @wking is on vacation, created this PR to continue work here.

Stubbing out how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279.

CC @wking, @LalatenduMohanty, @smarterclayton, @deads2k

@jottofar

Stubbing out how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. CC @jottofar, @LalatenduMohanty, @smarterclayton

jottofar · 2020-03-04T22:09:31Z

/cc @LalatenduMohanty Here's what I have thus far. Still need to fill in section "Mirroring to a central registry in a fully air gapped environment"

LalatenduMohanty · 2020-03-05T12:48:34Z

enhancements/oc/mirroring-manifests.md

+
+This filesystem can then be applied to the local registry with the given `oc image mirror` command.
+
+With this enhancement, the output format would gain an additional directory `manifests` under the base directory, a sibling to the current `v2`, containing manifest files that can be applied to the local cluster with:


This is little confusing as we already mentioning the manifest files in the command output before saying that this enhancement will introduce that or I misunderstood

We haven't yet mentioned manifest files in this section "Proposal-Pushing manifests to disk for later server application". The section starts out showing what the current 'oc adm release mirror' "push to disk" functionality is and what the resulting directory structure would look like. Then goes on to explain that the enhancement adds an additional "manifests" directory which will contain the newly outputted manifests. Does that make it clear?

yes, thanks

We already have the manifests keyword for a directory name in the exiting output. I would suggest we change the new manifest directory name to image-signatures or something similar.

We haven't yet mentioned manifest files in this section "Proposal-Pushing manifests to disk for later server application". The section starts out showing what the current 'oc adm release mirror' "push to disk" functionality is and what the resulting directory structure would look like. Then goes on to explain that the enhancement adds an additional "manifests" directory which will contain the newly outputted manifests. Does that make it clear?

We already have the manifests keyword for a directory name in the exiting output. I would suggest we change the new manifest directory name to image-signatures or something similar.

This brings up another related point. This enhancement is written to describe a general purpose manifest mirroring function which is fine. However our first implementation of this is to handle image signatures which requires specific logic, e.g. searching for specific strings to locate the signature information. Even if we do create a general purpose directory to contain general manifests the signature ConfigMap files intended for image verification should should be saved somewhere separate so they can be easily located as such.

Even if we do create a general purpose directory to contain general manifests the signature ConfigMap files intended for image verification should should be saved somewhere separate so they can be easily located as such.

The opacity of manifests is intentional. Users should not need to care what we have in that directory, which is why I had a "Users who intend to audit the gathered manifests..." non-goal in my 9517e1f. Folks who want to get signatures can iterate through our manifests looking for YAML ConfigMaps in the openshift-config-managed namespace with the release.openshift.io/verification-signatures label. They can use that same approach to find other manifest subsets if/when we add additional manifests in the future. But by using an opaque bucket for all the Kubernetes objects capturing external content for mirroring, we future-proof ourselves against changes in the actual objects that we capture, changes in directory structure, etc.

This is not an issue as user needs to explicitly give a path to store the signature manifests.

This is not an issue as user needs to explicitly give a path to store the signature manifests.

Why? They can apply with oc apply -Rf mirror/manifests, they don't need anything signature-specific for that.

@wking Right, that's why I said my initial concern is an non issue.

jottofar · 2020-03-06T22:53:43Z

WIP oc implementation here openshift/oc#343

LalatenduMohanty · 2020-03-10T10:06:58Z

enhancements/oc/mirroring-manifests.md

+### Non-Goals
+
+This proposal does not specify the types of manifests that are created.
+Users who intend to audit the gathered manifests should extract the gathered types from the manifests themselves.


Users who intend to audit the gathered manifests should extract the gathered types from the manifests themselves. IMO we do not need to mention this here.

LalatenduMohanty · 2020-03-10T10:07:18Z

enhancements/oc/mirroring-manifests.md

+
+## Proposal
+
+### Pushing manifests to disk for later server application


/server application/ application/

LalatenduMohanty · 2020-03-10T10:08:57Z

enhancements/oc/mirroring-manifests.md

+
+This filesystem can then be applied to the local registry with the given `oc image mirror` command.
+
+With this enhancement, the output format would gain an additional directory `manifests` under the base directory, a sibling to the current `v2`, containing manifest files that can be applied to the local cluster with:


We already have the manifests keyword for a directory name in the exiting output. I would suggest we change the new manifest directory name to image-signatures or something similar.

LalatenduMohanty · 2020-03-10T10:20:10Z

enhancements/oc/mirroring-manifests.md

+9. Perform the upgrade:
+	```console
+	$ oc adm upgrade --to-image REGISTRY/REPOSITORY/release@sha256:81154f5c03294534e1eaf0319bef7a601134f891689ccede5d705ef659aa8c92
+	```


The above workflow looks good to me, but @smarterclayton would be the right person to take final view.

/assign @smarterclayton

You'll need --allow-explicit-upgrade here (cf. this comment), unless you have separately pointed your cluster at a Cincinnati upstream which includes the edge you're trying to take here.

Also, I'd have expected /release to be part of REPOSITORY, but haven't actually run an oc image mirror ... to check.

LalatenduMohanty · 2020-03-10T17:11:53Z

enhancements/oc/mirroring-manifests.md

+This enhancement adds a new `--apply-manifests` option that when specified will apply manifests directly to the connected cluster rather than outputting them to disk.
+When the `--apply-manifests` option is specified a user can also optionally specify `--overwrite` which would cause the apply to bahave as the 'oc apply --overwrite' does currently. If the manifest exists on the cluster it will be updated.
+If the manifest exists on the cluster and `--overwrite` is not specified a warning will be displayed that the manifest could not be created.
+If `oc` encountered an error applying a manifest, it could optionally attempt to apply additional manifests before exiting non-zero, but would not be required to attempt any additional manifests.


but would not be required to attempt any additional manifests. makes it hard to understand the expected behavior. Can we rewrite it in a simpler way?

jottofar · 2020-03-11T15:17:58Z

Need @wking to fill in User Story In-cluster Cincinnati if this is really required. Is it?
All other applicable sections are now addressed. If a section does not apply or is not required I left the section heading but removed the FIXME. I checked other approved/merged oc enhancements and saw that it was common not to fill in all sections and in some cases the heading was left and in others the heading was removed. I feel leaving it better shows it was considered.

jottofar · 2020-03-11T16:12:33Z

/unhold

wking · 2020-03-12T18:24:59Z

enhancements/oc/mirroring-manifests.md

+For disconnected clusters, `oc adm release mirror ...` already helps users copy the release image and its dependencies into their local mirror. As of 4.2 and 4.3, the following command:
+
+```console
+$ oc adm release mirror \


Why are we promoting this up out of the Proposal section into the Summary? It seems like an implementation detail that we don't need for the high-level summary.

It helps explain the current impl and then I refer to the Summary in section "Pushing manifests to disk for later application" as part of explaining an ehancement need there

But why do we need to show an example oc command to explain the current implementation, especially just for this single images-to-disk case?

I do not see any harm showing the current implementation. At least it helps in better review of this PR.

I'm not saying "don't show the current implementation", I'm saying "this is not summary-level information, this is proposal-level background".

wking · 2020-03-12T18:29:50Z

enhancements/oc/mirroring-manifests.md

-
 [cvo-config-map-signatures]: https://github.com/openshift/cluster-version-operator/pull/279
-[custom-resource]: https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/
+[custom-resources]: https://kubernetes.io/docs/concepts/extend-kubernetes/api-extens`oc adm release mirror ...` commandion/custom-resources/


Is oc adm release mirror ... an accidental copy/paste issue?

wking · 2020-03-12T18:30:21Z

enhancements/oc/mirroring-manifests.md


 ### Graduation Criteria

-FIXME


I think we need to fill in content here, not just drop the FIXME.

LalatenduMohanty

/lgtm

wking · 2020-03-12T19:22:19Z

enhancements/oc/mirroring-manifests.md


 The `manifests` directory may or may not contain additional subdirectories, so the recursive `-R` should be given for future-proofing.

-### Pushing to disk while pushing containers to the mirror registry


I think we want to retain this header to cover this use-case.

Although maybe as ### Pushing manifests to disk while pushing containers to the mirror registry or some such.

This was part of my effort to get three sections per @smarterclayton comment. So I have the two sections "Pushing manifest to disk..." and "Applying manifests directly to the target cluster" but should still cover all use-cases. I think it flows better too, you push manifests to disk whether or not you're pushing containers to a mirror registry. The key point about the latter though is the need for the new --manifests-to-dir which is explained in that context.

wking · 2020-03-12T19:25:19Z

enhancements/oc/mirroring-manifests.md

+### Applying manifests directly to the target cluster
+
+This enhancement adds a new `--apply-manifests` option that when specified will apply manifests directly to the connected cluster rather than outputting them to disk.
+When the `--apply-manifests` option is specified a user can also optionally specify `--overwrite` which would cause the apply to bahave as the 'oc apply --overwrite' does currently. If the manifest exists on the cluster it will be updated.


nits: "bahave" -> "behave" and backticks instead of single-quotes for oc apply --overwrite. Also, two sentences on one line.

wking · 2020-03-12T20:27:11Z

enhancements/oc/mirroring-manifests.md

-
 ## Alternatives

-FIXME


Talk about serving signatures from Cincinnati (although I still think we need something attached to oc adm release mirror ... to gather external data for shipping into the cluster, even if that external data was a Cincinnati graph-with-signatures snapshot)?

wking · 2020-03-12T20:28:27Z

enhancements/oc/mirroring-manifests.md


-FIXME
-
 ## Infrastructure Needed [optional]


If we aren't going to request any infrastructure, we should probably drop this header as well. The only thing I can think of requesting would be long-lived VPCs if we wanted to add tests using that approach to blackholing CI-registry access.

@wking

NOTE: Since @wking is on vacation, created this PR to continue work here. Stubbing out how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. CC @wking, @LalatenduMohanty, @smarterclayton, @deads2k

openshift-ci-robot · 2020-03-12T20:48:16Z

New changes are detected. LGTM label has been removed.

LalatenduMohanty · 2020-03-13T07:53:36Z

enhancements/oc/mirroring-manifests.md

+$ oc apply -Rf mirror/manifests
+```
+
+The `manifests` directory may or may not contain additional subdirectories, so the recursive `-R` should be given for future-proofing.


Why can not make recursive behavior as the default. Less CLI flags = improved user experience.

Why can not make recursive behavior as the default. Less CLI flags = improved user experience.

Adjusting oc apply is orthogonal to this enhancement ;).

LalatenduMohanty · 2020-03-13T07:57:47Z

enhancements/oc/mirroring-manifests.md

+#### In-cluster Cincinnati
+
+Clusters running within a disconnected network will run Cincinnati on premise to provide an upgrade experience much more like that found on a cluster running within a connected network.
+The `oc adm release mirror ...` command is expected to be used to ease the installation of an on premise Cincinnati by mirroring the Cincinnati images.


Do you mean Cincinnati images or the secondary metadata from cincinnati-graph-data repository?

Do you mean Cincinnati images or the secondary metadata from cincinnati-graph-data repository?

Both. In my revived #188, I've made this "mirroring the Cincinnati images and manifests" to make that clearer.

LalatenduMohanty · 2020-03-13T08:10:43Z

enhancements/oc/mirroring-manifests.md

-FIXME
+The image is included in the payload, but has no content running in a cluster to upgrade.

 ### Version Skew Strategy


+1, We need to address

* user uses 4.2 `oc` binary to run copy images `oc adm release mirror --to-dir=` to a directory * updates to 4.5 `oc` binary and runs the --apply-manifests command. So CVO does not get the signature manifests

With respect to future version where we have a new manifest , we need to update oc command to check if the manifest is present and error out accordingly. That's why the structure of manifest directory is important as it should have room for future enhancements.

openshift-ci-robot · 2020-03-13T08:12:32Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jottofar
To complete the pull request process, please assign smarterclayton
You can assign the PR to them by writing /assign @smarterclayton in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

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

wking · 2020-03-15T22:04:00Z

I think I've picked up most of the changes here and folded them back into my revived #188 (and also filled in the remaining FIXMEs and fleshed some things out a bit more). Moving the conversation back over to #188...

/close

openshift-ci-robot · 2020-03-15T22:04:24Z

@wking: Closed this PR.

Details

In response to this:

I think I've picked up most of the changes here and folded them back into my revived #188 (and also filled in the remaining FIXMEs and fleshed some things out a bit more). Moving the conversation back over to #188...

/close

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.

Describing how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. Personally, I prefer 'manifests' naming here, but there was some concern that the Kubernetes/OpenShift manifests would be confused with the container image manifests in v2/openshift/release/manifests [1,2]. So I've used very explicit kubernetes-manifests, --apply-kubernetes-manifests, and --kubernetes-manifests-to-dir instead of my preferred manifests, --apply-manifests, and --manifests-to-dir. [1]: openshift#238 (comment) [2]: openshift#188 (comment)

Describing how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. Personally, I prefer 'manifests' naming here, but there was some concern that the Kubernetes/OpenShift manifests would be confused with the container image manifests in v2/openshift/release/manifests [1,2]. So I've used very explicit kubernetes-manifests, --apply-kubernetes-manifests, and --kubernetes-manifests-to-dir instead of my preferred manifests, --apply-manifests, and --manifests-to-dir. The truncated digest hash in the example signature filename avoids any concerns about filename length limitations on restrictive operating systems or filesystems. [1]: openshift#238 (comment) [2]: openshift#188 (comment) [3]: openshift#188 (comment)

Describing how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. Personally, I prefer 'manifests' naming here, so: * manifests directory * --apply-manifests and --manifests-to-dir options. But there was some concern that the Kubernetes/OpenShift manifests would be confused with the container image manifests in v2/openshift/release/manifests [1]. kubernetes-manifests was suggested [2], which would to a kubernetes-manifests directory and --apply-kubernetes-manifests and --kubernetes-manifests-to-dir options. But there was some concern about using Kubernetes anywhere [3], so the commit goes with a config directory and --apply-config and --config-to-dir. The truncated digest hash in the example signature filename avoids any concerns about filename length limitations on restrictive operating systems or filesystems [4]. [1]: openshift#238 (comment) [2]: openshift#188 (comment) [3]: openshift#188 (comment) [4]: openshift#188 (comment)

Describing how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. Personally, I prefer 'manifests' naming here, so: * manifests directory * --apply-manifests and --manifests-to-dir options. But there was some concern that the Kubernetes/OpenShift manifests would be confused with the container image manifests in v2/openshift/release/manifests [1]. kubernetes-manifests was suggested [2], which would to a kubernetes-manifests directory and --apply-kubernetes-manifests and --kubernetes-manifests-to-dir options. But there was some concern about using Kubernetes anywhere [3], so the commit goes with a config directory and --apply-config and --config-to-dir options. The truncated digest hash in the example signature filename avoids any concerns about filename length limitations on restrictive operating systems or filesystems [4]. [1]: openshift#238 (comment) [2]: openshift#188 (comment) [3]: openshift#188 (comment) [4]: openshift#188 (comment)

…ncement Describing how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. Personally, I prefer 'manifests' naming here, so: * manifests directory * --apply-manifests and --manifests-to-dir options. But there was some concern that the Kubernetes/OpenShift manifests would be confused with the container image manifests in v2/openshift/release/manifests [1]. kubernetes-manifests was suggested [2], which would to a kubernetes-manifests directory and --apply-kubernetes-manifests and --kubernetes-manifests-to-dir options. But there was some concern about using Kubernetes anywhere [3], so the commit goes with a config directory and --apply-config and --config-to-dir options. The truncated digest hash in the example signature filename avoids any concerns about filename length limitations on restrictive operating systems or filesystems [4]. [1]: openshift#238 (comment) [2]: openshift#188 (comment) [3]: openshift#188 (comment) [4]: openshift#188 (comment)

This PR implements the current baseline of openshift/enhancements#238 (which is a continuation of the work originated here: openshift/enhancements#188) Local package pkg/verify was reused with minor changes from CVO so there is PR openshift/library-go#725 to move package verify into library-go.

Describing how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. Personally, I prefer 'manifests' naming here, so: * manifests directory * --apply-manifests and --manifests-to-dir options. But there was some concern that the Kubernetes/OpenShift manifests would be confused with the container image manifests in v2/openshift/release/manifests [1]. kubernetes-manifests was suggested [2], which would to a kubernetes-manifests directory and --apply-kubernetes-manifests and --kubernetes-manifests-to-dir options. But there was some concern about using Kubernetes anywhere [3], so the commit goes with a config directory and options which leave the format implicit (--apply-release-image-signature and --release-image-signature-to-dir). The truncated digest hash in the example signature filename avoids any concerns about filename length limitations on restrictive operating systems or filesystems [4]. The narrow scoping vs openshift#188 accomodates pushback from Clayton in [5] and similar out of band discussion. [1]: openshift#238 (comment) [2]: openshift#188 (comment) [3]: openshift#188 (comment) [4]: openshift#188 (comment) [5]: openshift#188 (comment)

Describing how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. Personally, I prefer 'manifests' naming here, so: * manifests directory * --apply-manifests and --manifests-to-dir options. But there was some concern that the Kubernetes/OpenShift manifests would be confused with the container image manifests in v2/openshift/release/manifests [1]. kubernetes-manifests was suggested [2], which would to a kubernetes-manifests directory and --apply-kubernetes-manifests and --kubernetes-manifests-to-dir options. But there was some concern about using Kubernetes anywhere [3], so the commit goes with a config directory and options which leave the format implicit (--apply-release-image-signature and --release-image-signature-to-dir). The truncated digest hash in the example signature filename avoids any concerns about filename length limitations on restrictive operating systems or filesystems [4]. The narrow scoping vs openshift#188 accommodates pushback from Clayton in [5] and similar out of band discussion. [1]: openshift#238 (comment) [2]: openshift#188 (comment) [3]: openshift#188 (comment) [4]: openshift#188 (comment) [5]: openshift#188 (comment)

Describing how we're going to feed restricted-network clusters the release-signature ConfigMaps that the cluster-version operator reads since openshift/cluster-version-operator#279. Personally, I prefer 'manifests' naming here, so: * manifests directory * --apply-manifests and --manifests-to-dir options. But there was some concern that the Kubernetes/OpenShift manifests would be confused with the container image manifests in v2/openshift/release/manifests [1]. kubernetes-manifests was suggested [2], which would to a kubernetes-manifests directory and --apply-kubernetes-manifests and --kubernetes-manifests-to-dir options. But there was some concern about using Kubernetes anywhere [3], so the commit goes with a config directory and options which leave the format implicit (--apply-release-image-signature and --release-image-signature-to-dir). The truncated digest hash in the example signature filename avoids any concerns about filename length limitations on restrictive operating systems or filesystems [4]. The narrow scoping vs openshift#188 accommodates pushback from Clayton in [5] and similar out of band discussion. The two-releases of backwards compatibility is based on [6]. I'm extrapolating from Clayton's comment to assume that "releases" means 4.(y+2), not 4.y -> 6.0. [1]: openshift#238 (comment) [2]: openshift#188 (comment) [3]: openshift#188 (comment) [4]: openshift#188 (comment) [5]: openshift#188 (comment) [6]: openshift#283 (comment)

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 4, 2020

openshift-ci-robot requested review from soltysh and timlnx March 4, 2020 22:09

LalatenduMohanty mentioned this pull request Mar 5, 2020

enhancements/oc/mirroring-configuration-manifests: Propose a new enhancement #188

Closed

LalatenduMohanty reviewed Mar 5, 2020

View reviewed changes

jottofar mentioned this pull request Mar 6, 2020

Implement enhancements/oc/mirroring-release-signatures openshift/oc#343

Merged

jottofar force-pushed the oc-mirror-manifests branch 2 times, most recently from bd48136 to d26cda1 Compare March 9, 2020 21:57

LalatenduMohanty reviewed Mar 10, 2020

View reviewed changes

LalatenduMohanty suggested changes Mar 10, 2020

View reviewed changes

LalatenduMohanty reviewed Mar 10, 2020

View reviewed changes

jottofar force-pushed the oc-mirror-manifests branch from d26cda1 to 5c0e043 Compare March 11, 2020 15:10

openshift-ci-robot assigned smarterclayton Mar 11, 2020

jottofar force-pushed the oc-mirror-manifests branch from 5c0e043 to 25c3888 Compare March 11, 2020 16:08

jottofar changed the title ~~WIP: enhancements/oc/mirroring-manifests: Propose a new enhancement~~ Enhancements/oc/mirroring-manifests: Propose a new enhancement Mar 11, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 11, 2020

wking reviewed Mar 12, 2020

View reviewed changes

LalatenduMohanty approved these changes Mar 12, 2020

View reviewed changes

openshift-ci-robot assigned LalatenduMohanty Mar 12, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2020

wking reviewed Mar 12, 2020

View reviewed changes

jottofar force-pushed the oc-mirror-manifests branch from 25c3888 to 2b80338 Compare March 12, 2020 20:48

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Mar 12, 2020

LalatenduMohanty suggested changes Mar 13, 2020

View reviewed changes

openshift-ci-robot closed this Mar 15, 2020


		This filesystem can then be applied to the local registry with the given `oc image mirror` command.

		With this enhancement, the output format would gain an additional directory `manifests` under the base directory, a sibling to the current `v2`, containing manifest files that can be applied to the local cluster with:


		## Proposal

		### Pushing manifests to disk for later server application


		The `manifests` directory may or may not contain additional subdirectories, so the recursive `-R` should be given for future-proofing.

		### Pushing to disk while pushing containers to the mirror registry


		## Alternatives

		FIXME

Enhancements/oc/mirroring-manifests: Propose a new enhancement #238

Enhancements/oc/mirroring-manifests: Propose a new enhancement #238

Uh oh!

Conversation

jottofar commented Mar 4, 2020

Uh oh!

jottofar commented Mar 4, 2020

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

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!

jottofar commented Mar 6, 2020

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

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!

jottofar commented Mar 11, 2020

Uh oh!

jottofar commented Mar 11, 2020

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LalatenduMohanty left a comment

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

wking Mar 12, 2020 •

edited

Loading