Skip to content

Conversation

@jc-berger
Copy link
Contributor

@jc-berger jc-berger commented Feb 10, 2021

@openshift-ci-robot 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 Feb 10, 2021
@netlify
Copy link

netlify bot commented Feb 10, 2021

Deploy preview for osdocs ready!

Built with commit 9908ec9

https://deploy-preview-29411--osdocs.netlify.app

@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch from 3bea5c8 to cca95ee Compare February 18, 2021 02:00
@jc-berger
Copy link
Contributor Author

@Preeticp, new commit is up, addressing your feedback! I'll look into the build failure tomorrow...I suspect it's because of a doc I've deleted (outdated module). Thanks for the help. Now I'll await the dev SME review!

@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch 3 times, most recently from 1e9754f to 745d535 Compare February 18, 2021 18:15
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 18, 2021
@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch from 745d535 to a5204c9 Compare February 18, 2021 21:53
@jc-berger
Copy link
Contributor Author

@Preeticp and @sbose78, thank you both for the awesome review :)

The big issue I have is that I see this branch has conflicts and I am unable to rebase my branch because of the conflicting _topic_map.yml. Either of you know what could be causing this issue?

@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 18, 2021
@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch 2 times, most recently from 0496300 to e54c02f Compare February 19, 2021 13:14
@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch from e54c02f to 8a77b12 Compare February 19, 2021 15:08
@sbose78
Copy link

sbose78 commented Feb 19, 2021

Looks good to me!

@jc-berger jc-berger changed the title WIP: created getting started wtih gitops procedure module Created getting started wtih gitops procedure module Feb 19, 2021
@openshift-ci-robot 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 Feb 19, 2021
Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like two different user stories wedged into the same assembly, you have some malformed modules, and I am confused about part of the flow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
After you create a new client in Keycloak and generate an *argocd-secret* in the Argo CD secret, you get a client secret. To configure Argo CD OIDC, store this client secret with the following instructions:
. Configure Argo CD OpenID Connect (OIDC) to connect to your Keycloak instance:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
. Encode the client secret in base64:
.. Encode the client secret in base64:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$ echo -n '83083958-8ec6-47b0-a411-a8c55381fbd2' | base64
$ echo -n '<argocd-secret>' | base64 <1>

Then after the codeblock, add something like:

<1> Enter the `<argocd-secret>` value that you obtained from your Keycloak server.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this point on, you've completely lost me. The descriptions of the steps don't appear to correspond to the commands that you're using, and I am extremely confused what of the YAML you're showing as example is actually there and what the user needs to customize. The YAML format itself looks slightly off - I've never seen a YAML example start with a "$." Please review the rest of the module and try to be clearer. Use the annotation format from some of the other YAML examples to explain what's going on.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not make this a separate step. I'd make these checks annotations on the YAML itself. This format is hard to follow.

@kalexand-rh kalexand-rh added peer-review-done Signifies that the peer review team has reviewed this PR branch/enterprise-4.7 labels Feb 19, 2021
@kalexand-rh kalexand-rh added this to the Future Release milestone Feb 19, 2021
@vikram-redhat vikram-redhat added the dev-tools Label for all Odo/Pipelines/Helm/Developer Console/Perspective PRs label Feb 20, 2021
@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch from 8a77b12 to d2a09ef Compare February 22, 2021 17:08
@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 22, 2021
@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch from d2a09ef to a90062a Compare February 22, 2021 17:36
@Preeticp
Copy link
Contributor

Hi @kalexand-rh thanks a ton for your thorough review, appreciate it.
Post discussions with the dev team, we thought it best to add in a brief getting started for the TP release, and then add in the other changes incrementally.
We have removed the: modules/setting-up-an-openshift-login.adoc and the modules/setting-up-a-customized-argocd-instance.adoc from this PR, and have created Jiras to fix them post GA: https://issues.redhat.com/browse/RHDEVDOCS-2695 and https://issues.redhat.com/browse/RHDEVDOCS-2696. The Jiras have links to the relevant comment for both modules so that we have a record and fix all the comments.

@dewan-ahmed
Copy link

Hi, few comments:

  1. On step 3d. Select the argo-cluster-cluster instance to display the password., I think the name of the password is argocd-cluster-cluster
    access-secret-1
  2. From what I read, the getting started doc is guiding the customer up to the running ArgoCD instance. Was that the entire scope of the getting started doc? My take on this is providing more information to a customer i.e. how to make use of that instance in terms of creating a sample argocd application. Since this is a new product and not so much content exist on this online, these docs would be the only go-to places so my preference is to provide a bit more info. @sbose78 @wtam2018 can comment further.

@sbose78
Copy link

sbose78 commented Feb 22, 2021

Getting started definitely needs to involve

  • Installing OpenShift GitOps/ArgoCD as an "admin".
  • Enabling users/admins to deploy applications with it.

We don't need all of it to go in together though :)

@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch from a90062a to 07ec9f6 Compare February 22, 2021 18:44
corrected metadata, module structure, and minor content changes

revised content, made modules and assembly

corrected typo to remove build error

changed xref to resolve build error

added module to include openshift login

content and structure changes to modules and assembly

fixed xref to resolve build error

fixed xref to resolve build error#

more content and heading changes

modular and structural changes

changed introduction to module

minor grmmatical changes

corrected UI elements
@jc-berger jc-berger force-pushed the jcberger_2475_gettingstartedgitops branch from 07ec9f6 to 9908ec9 Compare February 22, 2021 18:45
Copy link

@sbose78 sbose78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this looks good and has a much better scope for a "Getting started" topic!

@codyhoag codyhoag merged commit d5ffdb2 into openshift:master Feb 22, 2021
@codyhoag
Copy link
Contributor

/cherrypick enterprise-4.7

@openshift-cherrypick-robot

@codyhoag: new pull request created: #29661

In response to this:

/cherrypick enterprise-4.7

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.7 dev-tools Label for all Odo/Pipelines/Helm/Developer Console/Perspective PRs peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants