Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update commcare cloud setup instructions #6458

Merged
merged 9 commits into from
Jan 20, 2025
Merged

Update commcare cloud setup instructions #6458

merged 9 commits into from
Jan 20, 2025

Conversation

gherceg
Copy link
Contributor

@gherceg gherceg commented Jan 3, 2025

I've run into various issues while setting up monoliths and am attempting to clarify those instructions in this PR.

This also notably updates documentation to be aligned with our removal of the suggestion to copy a private key to the server, and instead use ssh -A when/where appropriate.

Review by commit.

Environments Affected

None

This is necessary so that COMMCARE_CLOUD_ENVIRONMENTS environment
variable is set properly before running init-ansible.
bootstrap-users sets up the expected ssh config based on public keys
previously added in these instructions. Once run, the user should then
exit and re-ssh into the machine using the -A option to ensure agent
forwarding is enabled.
This is removed once bootstrap-users is run
@gherceg gherceg changed the title Gh/docs/install Update commcare cloud setup instructions Jan 3, 2025

$ sudo update-alternatives --install /usr/bin/python python /usr/bin/python3 10


Copy link
Contributor

Choose a reason for hiding this comment

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

is this not needed anymore?

I used this one recently when trying to do a test install.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, its moved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that we specify ubuntu 22.04 at the moment, 3.10 will be the system version, so it seems fine to simplify the step to just run this rather than specify the condition.

@@ -111,6 +111,12 @@ reference.

PasswordAuthentication yes

To allow keyboard interactive authentication, ensure
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this is needed in specific cases and not all interactive authentication. I definitely could do an install without this change.

Found this useful reference: https://serverfault.com/questions/189643/sshd-blocking-password-authentication

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah agreed it isn't necessary all of the time. Just checking that you agree that it isn't worth specifying that in the instructions given that it will be reverted in a later setup step anyway?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think if someone gets stuck at this step like you did, they are doing something special.

Seems fine to leave it if you think it could be useful for someone to make the initial root password step work, with a comment that "If needed you can enable keyboard interactive authentication with", though there are so many steps here that one would just go about copying pasting commands without really reading the instructions well, which is why it would be nice to keep commands limited and exclude anything special.
So, This could go either also go here

Agreed that this later gets cleaned up.


::

$ eval `ssh-agent`
$ ssh-add ~/.ssh/id_rsa
$ commcare-cloud cluster bootstrap-users
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@AmitPhulera AmitPhulera left a comment

Choose a reason for hiding this comment

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

Great improvements, clearly makes things lot clearer. Thanks for the PR. Just have a few questions/suggestions otherwise this looks gtg.

docs/source/installation/2-manual-install.rst Outdated Show resolved Hide resolved
@@ -467,13 +464,13 @@ Install CommCare Cloud

$ cp ~/commcare-cloud/src/commcare_cloud/config.example.py ~/commcare-cloud/src/commcare_cloud/config.py

Update the known hosts file
Update the known hosts file (substituting your environment name if necessary)

::

$ commcare-cloud cluster update-local-known-hosts
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we have used cluster as environment name in the entire doc but would it be clearer if we use which would indicate that this has to be replaced?

Suggested change
$ commcare-cloud cluster update-local-known-hosts
$ commcare-cloud <env> update-local-known-hosts

btw I am totally fine with the way it is right now it was just a thought that I wanted to share with you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I do want to make this change. I also want to make a sweeping change to the $ syntax before each command as it makes copy/pasting annoying, so was planning to do this in a separate PR if that is ok with you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah. Totally fine by me.

Comment on lines 453 to 459
$ nano ~/.profile

Paste the following before the line that sources the init-ansible script.

::

source ~/.commcare-cloud/load_config.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Any specific case where this approach better than the previous one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the order of instructions, load_config.sh was being run after init-ansible, but load_config plays a critical role in exporting the COMMCARE_CLOUD_ENVIRONMENTS variable to point to the right environments directory. I know this change complicates the setup a bit, but hopefully results in fewer issues down the road. Does that answer your question?

Copy link
Contributor

Choose a reason for hiding this comment

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

The "before ... init-ansible" instruction appears to contradict the output of manage-commcare-cloud configure, which says to append to ~/.profile.

puts(color_notice("Append the following to your ~/.profile:"))
puts(color_code("source ~/.commcare-cloud/load_config.sh"))

load_config.sh was being run after init-ansible, but load_config plays a critical role in exporting the COMMCARE_CLOUD_ENVIRONMENTS variable to point to the right environments directory

Does that order matter? That is, does init-ansible need COMMCARE_CLOUD_ENVIRONMENTS to be set before it is run?

Copy link
Contributor

Choose a reason for hiding this comment

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

Another option would be to have the init-ansible script source ~/.commcare-cloud/load_config.sh if it exists, and skip the instruction to add it to .profile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah upon closer inspection, I may have incorrectly assumed this was the cause of an issue I was facing. It seems fair to say that the init-ansible script does not use the COMMCARE_CLOUD_ENVIRONMENTS variable for anything other than how it is used in update_code.sh. I think I know where I went wrong, but I'll try to run through the setup steps without this change just to make sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok it seems fine. Reverted in 17240b4.

@gherceg
Copy link
Contributor Author

gherceg commented Jan 6, 2025

Possibly a more extreme change, but I'm tempted to remove support for running bootstrap-users as part of the deploy-stack command entirely, since bootstrap users is a one-off command that will change the state of user permissions on the system, and inevitably result in needed to re-run deploy-stack from a different user config, at least in my experience.

Comment on lines 453 to 459
$ nano ~/.profile

Paste the following before the line that sources the init-ansible script.

::

source ~/.commcare-cloud/load_config.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

The "before ... init-ansible" instruction appears to contradict the output of manage-commcare-cloud configure, which says to append to ~/.profile.

puts(color_notice("Append the following to your ~/.profile:"))
puts(color_code("source ~/.commcare-cloud/load_config.sh"))

load_config.sh was being run after init-ansible, but load_config plays a critical role in exporting the COMMCARE_CLOUD_ENVIRONMENTS variable to point to the right environments directory

Does that order matter? That is, does init-ansible need COMMCARE_CLOUD_ENVIRONMENTS to be set before it is run?

@gherceg gherceg merged commit b03d359 into master Jan 20, 2025
2 checks passed
@gherceg gherceg deleted the gh/docs/install branch January 20, 2025 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants