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

Documentation for general installation in containerized environments #3355

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

me-coder
Copy link
Contributor

@me-coder me-coder commented Oct 2, 2024

The CFEngine installation guide is missing out on containers.
This PR is an attempt to project the capability of CFEngine with containers (using ubi9-init images).

There are a few limitations due to the nature of CFEngine (requires privileged mode and container is required to have systemd, which need initialization with /var/sbin/init).
However, it still gives a good ability to test CFEngine in containerized environments in full capacity, without the need to have a bulky multi VM setup.

Hope the moderator team finds this useful and decides to integrate it into the docs.

@me-coder me-coder force-pushed the master branch 5 times, most recently from fa645e7 to 5e7da00 Compare October 2, 2024 19:18
@me-coder
Copy link
Contributor Author

me-coder commented Oct 2, 2024

I am unable to figure out the reason for failure of check with:
Run markdowner.py on all markdown files and see if there are changes / check (pull_request)

Any help to deal with this cryptic error is appreciated.

@olehermanse
Copy link
Member

I am unable to figure out the reason for failure of check with:
Run markdowner.py on all markdown files and see if there are changes / check (pull_request)

Any help to deal with this cryptic error is appreciated.

Your files have extra spaces in them, at the ends of multiple lines.
Change your editor to automatically trim trailing whitespace for you, or run the command as suggested;

python3 .github/workflows/markdowner.py ./getting-started/installation/general-installation/installation-community-containerized.markdown

That will edit the file, removing those extra unnecessary spaces.
(Afterwards you will need to add and commit those changes - git add -p ; git commit).

@me-coder
Copy link
Contributor Author

me-coder commented Oct 3, 2024

I am unable to figure out the reason for failure of check with:
Run markdowner.py on all markdown files and see if there are changes / check (pull_request)

Any help to deal with this cryptic error is appreciated.

Your files have extra spaces in them, at the ends of multiple lines. Change your editor to automatically trim trailing whitespace for you, or run the command as suggested;

python3 .github/workflows/markdowner.py ./getting-started/installation/general-installation/installation-community-containerized.markdown

That will edit the file, removing those extra unnecessary spaces. (Afterwards you will need to add and commit those changes - git add -p ; git commit).

The extra spaces (particularly ^.+<space><space>$ was required for newline as per the markdown requirements.
However, that has been replaced by </br> now and seems to have worked.
Thanks for the suggestion.

@me-coder
Copy link
Contributor Author

me-coder commented Oct 3, 2024

@olehermanse,
I've made this change to take care of such situations, if encountered by other contributors:
c9719c9

Hope that's fine.

@olehermanse
Copy link
Member

The extra spaces (particularly ^.+$ was required for newline as per the markdown requirements.

What requirements? This is not required.

However, that has been replaced by
now and seems to have worked.

No, remove these, they are not necessary.

I've made this change to take care of such situations, if encountered by other contributors:
c9719c9

Hope that's fine.

No, that change is not correct, remove it. We want to strip trailing whitespace. You don't need trailing whitespace nor html tags (</br>) to write this tutorial.

@me-coder me-coder requested a review from olehermanse October 4, 2024 19:16
@me-coder
Copy link
Contributor Author

me-coder commented Oct 4, 2024

Hi @olehermanse, @nickanderson,
Thank you for the detailed review.

I made the suggested changes and also a few enhancements to the document and force pushed it as a amended single commit.
I'm not sure if the suggested commits were supposed to be incorporated or were just markers for the review suggestions. (unaware of the rules being followed for this project).

I submitted a single commit as I always feel it's cleaner, could be fast-forwarded and there were changes which I could easily edit as a batch in VSCode editor.

Kindly re-consider the review with the applied changes.

- @me-coder

@nickanderson nickanderson dismissed their stale review October 10, 2024 04:28

Changes have been made, and re-review is needed.

@nickanderson
Copy link
Member

I am traveling and I likely won't get around to looking at this again until next week at the very earliest if I manage to catch up with other things after travel.

@me-coder
Copy link
Contributor Author

Hi @nickanderson , @olehermanse, @craigcomstock,
Could you revisit the review?

@craigcomstock
Copy link
Contributor

Hi @nickanderson , @olehermanse, @craigcomstock, Could you revisit the review?

Sorry I missed this. We are a bit busy with release right now so I will likely get to this in a week or so.

@me-coder
Copy link
Contributor Author

Hi @nickanderson , @olehermanse, @craigcomstock, Could you revisit the review?

Sorry I missed this. We are a bit busy with release right now so I will likely get to this in a week or so.

No worries, just wanted to make sure this is not forgotten :)

Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

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

Thanks again @me-coder for the submission and sorry it has taken a while!

I am going to work through the steps you gave but adjust to using our quick install script: https://cfengine.com/downloads/quick-install/

Will edit some comments with suggested changes today I hope.

@@ -2,7 +2,7 @@
layout: default
title: Installing Enterprise on CoreOS
Copy link
Contributor

Choose a reason for hiding this comment

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

We have deprecated support for CoreOS as of 3.24.x series so we should likely edit this page to specify only available in 3.21.x LTS and after that will be unsupported.

@craigcomstock
Copy link
Contributor

@me-coder sorry, in trying to push up three new commits with changes I may have pushed a bit too much. I will see if this needs fixing up (I think it does).

@craigcomstock
Copy link
Contributor

@me-coder I just needed to rebase. I have force pushed to your branch with your original commit and 3 new ones by me with proposed changes. Please take a look and see if things look OK to you.

Thanks again for the work!

@craigcomstock
Copy link
Contributor

Ah. I didn't test docker compose bits yet so will do that next to make sure all is well there. Will do that a bit later today, a few hours from now I should be done with that and post results/fixes here.

@craigcomstock
Copy link
Contributor

@me-coder oops, I will rework the hub/server, agent/host changes as I have been informed we don't want to make that distinction.

@craigcomstock
Copy link
Contributor

craigcomstock commented Dec 10, 2024

@me-coder oops, I will rework the hub/server, agent/host changes as I have been informed we don't want to make that distinction.

Fixed up. I still need to test docker compose steps but this is ready for your review now otherwise @me-coder.

Copy link
Contributor

@craigcomstock craigcomstock left a comment

Choose a reason for hiding this comment

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

@me-coder can't thank you enough for this contribution. Very cool.

I tested the docker compose bits and they worked fine!

If you like the way things are with my changes you can squash the commits and we can merge.

I would say just remove the changes to the coreos md file? I am not sure why those changes were included in the first place?

@me-coder
Copy link
Contributor Author

If you like the way things are with my changes you can squash the commits and we can merge.

cf-remote install --edition community --clients localhost
This change is awesome. Technically this makes it distro independent.
Thanks for making this change and sharing this, it would be really useful in future when I am setting up cfengine. 👌

@me-coder
Copy link
Contributor Author

If you like the way things are with my changes you can squash the commits and we can merge.

cf-remote install --edition community --clients localhost
This change is awesome. Technically this would make it distro independent, if I understand what it does.

Thanks for making this change and sharing, it would be really useful in future when I am setting up cfengine. 👌
I've squashed the commits.

I would say just remove the changes to the coreos md file?

Done.

Thanks for your support!

@craigcomstock
Copy link
Contributor

If you like the way things are with my changes you can squash the commits and we can merge.

cf-remote install --edition community --clients localhost This change is awesome. Technically this would make it distro independent, if I understand what it does.

Yes. It does handle Windows and Linux at this point so quite helpful. I use it a lot!

@@ -2,7 +2,7 @@
layout: default
title: Installing Community
published: true
sorting: 50
Copy link
Contributor

Choose a reason for hiding this comment

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

@me-coder was this a suggestion from someone on our team? @olehermanse @nickanderson other than this change that doesn't seem needed I think we are good to commit!

Copy link
Contributor Author

@me-coder me-coder Dec 20, 2024

Choose a reason for hiding this comment

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

@craigcomstock,
This wasn't a suggestion from the team. The parameter said sorting and I found there was an overlap amongst the files regarding the numbering. Assuming it to be a required indexing, I had changed it.

@olehermanse,
If the approval was awaiting this change, I have reverted the change from this PR.

Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

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

I didn't test it myself, but i read it and it reads well. Thank you very much.

Adjusted instructions to use cf-remote and lts version/tag instead of 3.24.0-1
@craigcomstock
Copy link
Contributor

@me-coder outstanding work here, thanks so much for your contribution and your patience and persistence during review! Merging! :)

@craigcomstock craigcomstock merged commit aa628d4 into cfengine:master Dec 20, 2024
2 checks passed
@nickanderson
Copy link
Member

I didn't test it myself, but i read it and it reads well. Thank you very much.

Seconded!

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

Successfully merging this pull request may close these issues.

4 participants