📖 Adding compute bind and workspaces to readme

[salaboy]

Summary

📖 updating main getting started guide with required compute bind and creating two separate workspaces for locations and myapps.

@davidfestal thanks for the help .. these instructions are now working on main.

Related issue(s)

Fixes #

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign stevekuznetsov for approval by writing /assign @stevekuznetsov in a comment. For more information see the Kubernetes Code Review Process.

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

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

[openshift-ci]

Hi @salaboy. Thanks for your PR.

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

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

I understand the commands that are listed here.

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

[merlante]

I don't think it's clear why you now need to care about workspaces for this "hello world" scenario. In the existing quickstart everything happens in the default workspace ("root" I guess). (I'm actually having trouble using the existing quickstart with main now, but it works for v0.9.1.)

In this new quickstart, two workspaces are created, and it's not really clear why they are needed. It's easy to think kcp -> pcluster, but the user needs more context to figure out myapps -> location -> pcluster.

Also, why "location" and why "myapps"? I know you don't want to complicate things on the main README, but I think it might be confusing as is.

[salaboy]

@merlante I was hitting issues with the readme and this was suggested by @davidfestal , and now the readme works out of the box. I think that the intention is to have a clear separation between where you are going to deploy workloads and other workspaces that are more for admin stuff.
I was thinking about diagrams.. this getting started guide is really complex for new people and some diagrams might help a lot.

[merlante]

Hi @salaboy, I hit issues with the readme as well with the main branch, and so I agree it needs to be changed. :) I understand (mostly) the reason for the two workspaces, but I'm concerned that an average user coming to the site will have no idea. What is there right now might have issues, but at least it's easy enough to grasp. With these changes, we're moving to something more correct, but that is not completely intuitive or easy to understand.

One option would be to maybe get this PR merged in it's current form -- assuming that the steps are fully working (I will check today if I get the chance) -- and then add a separate PR afterwards to add disagrams/context.

[salaboy]

@merlante I commit to add the diagram to explain what is going on in a separate PR, if you manage to check that the steps here are working, let me know. Notice that this PR also add the KIND cluster creation command, without that is was a bit too confusing for a newbie like me.

[pdettori]

I tend to agree with @merlante that for a Quickstart we should only document the minimum number of steps to allow users to kick the tires. On the other hand, it could be useful to explain scenarios where the synctarget and location are created in a user-created workspace and consumed from another user-created workspace (btw, I have been exercising that scenario as well using the main branch and run into this issue. I would suggest to leave the Quickstart as simple as possible, and add readme for the two workspaces scenarios or more advanced use cases under the "Next Steps" section.

[davidfestal]

/ok-to-test

[davidfestal]

We should probably not use the user home, since it makes the full name of the workspaces harder to grasp: root:users:zu:yc:kcp-admin:locations.
As the kcp-admin user, we have the permission to create workspaces directly at the root level.

[davidfestal]

The bind compute command should be done from inside the new workspace used to deploy applications.

[salaboy]

@davidfestal I think because of the flow that's the case.. running the steps as described here was working as expected.

[davidfestal]

So this PR had been left unmerged until now, partly due to the discussion found here.

But after this slack comment on the community channel, it seems it could still be worth merging.

@pdettori @MikeSpreitzer Since you contributed PRs on this part of the Readme recently, would you like to review this one and provide your input before we rebase it and possibly merge it ?

@sttts @ncdc Do you have any objection to the fact that the main Readme would introduce the distinction between location workspaces and workload workspaces ?

cc @richardcase

[openshift-merge-robot]

PR needs rebase.

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.

[MikeSpreitzer]

1. Things have changed since this PR was created and initially reviewed, it needs updating and fresh review.
2. 📖 Brush up TMC quickstart #2722 also contributes needed updates to the quickstart and needs review.
3. I see plausible arguments for and against overloading the usage of the root workspace. In my opinion, it is no good to have a quickstart that leads people astray; the quickstart should use different workspaces for different purposes in what we expect is a typical fashion.

[davidfestal]

1. Things have changed since this PR was created and initially reviewed, it needs updating and fresh review.
2. book Brush up TMC quickstart #2722 also contributes needed updates to the quickstart and needs review.
3. I see plausible arguments for and against overloading the usage of the root workspace. In my opinion, it is no good to have a quickstart that leads people astray; the quickstart should use different workspaces for different purposes in what we expect is a typical fashion.

So maybe the way to go it to first finish the review of #2722, and have it merged, then rebase / update / review this one on top of it ?

[ncdc]

@salaboy @davidfestal now that 2722 has merged, would you like to proceed with this?

[salaboy]

@ncdc I would love to get this done, but let's check with @davidfestal, can you please give us your feedback? Should I change something else now besides rebasing this?

[ncdc]

I haven't reviewed it (yet). Was just going back through older PRs and checking in on them

[ncdc]

Note, the content you're editing has been moved to docs/content/index.md

[davidfestal]

@ncdc I would love to get this done, but let's check with @davidfestal, can you please give us your feedback? Should I change something else now besides rebasing this?

i personally still agree with the changes in this PR. Seems to me that the main question we have to answer before move forward is the one asked to @sttts and @ncdc in a previous comment:

Do you have any objection to the fact that the main Readme would introduce the distinction between location workspaces and workload workspaces ?

@pdettori Since you initially made this comment, do you agree with @MikeSpreitzer when he writes:

In my opinion, it is no good to have a quickstart that leads people astray; the quickstart should use different workspaces for different purposes in what we expect is a typical fashion.

[davidfestal]

Let's do a quick survey here:

• add a 👍 to this comment if you agree with introducing the distinction between location workspaces and workload workspaces in the quickstart
• add a 👎 to this comment if you prefer keeping the quickstart as simple as possible (doing everything in root) even though it may lead people astray from the typical fashion, and make the reason of the kcp bind compute call hard to understand.

[ncdc]

I would introduce the distinction and explain (briefly) why.

[pdettori]

@davidfestal as a matter of fact I found useful to have that distinction and I have used that approach in my medium blog

[davidfestal]

So according to the comments above, it seems that we would introduce the distinction between location workspaces and user workspaces and, as @ncdc suggested, explain (briefly) why.
@salaboy Do you want to rebase your PR on top of main, and then we'd add the brief explanation ?

[salaboy]

@davidfestal yes.. I will do that in the next few days.. I am currently in a company retreat

[davidfestal]

@davidfestal yes.. I will do that in the next few days.. I am currently in a company retreat

Thanks !

[mjudeikis]

/close
TMC related. archiving with tracking