Cameron-IPFSPodcasting / podcastnode-Python

Python IPFS Podcast Client for IPFSPodcasting.net
GNU General Public License v3.0
7 stars 4 forks source link

Refactoring of some components #2

Closed suorcd closed 2 years ago

suorcd commented 2 years ago

I tested this on a Ubuntu 22.04 VPS image.

Please test and let me know if changes are needed.

Cameron-IPFSPodcasting commented 2 years ago

It all looks good, but I'll test it to be sure. Thanks for the 2nd (or 3rd) set of eyes.

CaffeinatedDNB commented 2 years ago

I didn't test it on Ubuntu 22.04 LTS as various underlying system changes have taken place that I don't particular care for plus it's too new. Ubuntu 20.04.5 LTS (as of this writing in Sep 2022) is solid and still has 3 more years of updates from Canonical (until 2025.)

I went ahead and checked out Ubuntu Server 22.04 on a new VPS instance and the installation process got interrupted by a kernel update screen requiring several steps to get through. That doesn't happen in 20.04 LTS. That's why I was specific as to which OS version it was tested under. :sunglasses:

CaffeinatedDNB commented 2 years ago

@Cameron-IPFSPodcasting

I've uploaded a newly saved version of the script incorporating some of the suggestions made by "suorcd" and his contribution was noted at each modification.

I also removed sudo in my original script that I found towards the top. Example: sudo apt update -qq && sudo apt -y -qq upgrade (it didn't affect things either way as the user would be running as root in any event but it's better/cleaner to not have it unless absolutely necessary.

I added the following: Adding Universe repository in the event it's not active so fail2ban and related packages don't fail

add-apt-repository -y universe

Believe it or not, I've never used Github for any pull/merge requests so I'll rely on you to make those commits. :+1:

Once again, thank you @suorcd

2022-09-12-IPFS-Node-Installation.txt

suorcd commented 2 years ago

@CaffeinatedDNB What is the target audience, skill level, etc. for this script?

suorcd commented 2 years ago

I didn't test it on Ubuntu 22.04 LTS as various underlying system changes have taken place that I don't particular care for plus it's too new. Ubuntu 20.04.5 LTS (as of this writing in Sep 2022) is solid and still has 3 more years of updates from Canonical (until 2025.)

I went ahead and checked out Ubuntu Server 22.04 on a new VPS instance and the installation process got interrupted by a kernel update screen requiring several steps to get through. That doesn't happen in 20.04 LTS. That's why I was specific as to which OS version it was tested under. sunglasses

There are definitely some more jarring interface choices, but most of the new installs will most likely be on 22.04

CaffeinatedDNB commented 2 years ago

I'm a proponent of K.I.S.S. (Keep It Simple Stupid)

The goals and intended audience have been clearly spelled out. :-)

From the script with documentation I've provided:

To be done via Hosted VPS Web console or whatever console you're using; VM or dedicated computer.

Generally, many will be running their IPFS nodes from a VPS provider so read up on their documentation on configuring the firewal


From my initial post on:

https://github.com/Cameron-IPFSPodcasting/podcastnode-Python/issues/1

"Provide concise documentation for the would-be IPFS Node admin to have a new instance up and running in about 10-20 minutes time with limited knowledge and/or interaction. Code documentation provides insight as to what each step is doing."


I'm going to address your points individually:

"@CaffeinatedDNB Adding a check shouldn't cause issues, especially if the script fails for some reason and the user tries to run again."

Once again, there's no need to add a check as it's a brand new VPS instance. Even if the user runs the script again, the "adduser" command would simply state that the user exists and move along. A non-issue.


"@CaffeinatedDNB Using usermod -aG sudo ipfs is a bit more widely available, but adduser ipfs sudo would follow the previous use of adduser."

Either way would be fine. What counts is that the original command gets the job done and works on the tested versions of Ubuntu Server 20.04 / 22.04 and Debian 11.


"@CaffeinatedDNB The '--enable-gc' was from a previous version that i had found, if the current implementation doesn't need no reason not to remove it."

I addressed that point earlier. Using that flag causes garbage collection to happen once an hour and as the IPFS node grows in size would unnecessarily tax the system.


"@CaffeinatedDNB Seems like ExecStop might have a difference in behavior between 22.04 and older versions, I did not exeperience any issues when using it for testing; but there was at least one other user that said it caused some issues."

I came across it in testing as well which is why the final code I provided didn't include it. "ipfs shutdown" works just fine, is consistent and is available to the user IF the need arises.


"@CaffeinatedDNB As for the slightly odd way to invoke the ipfs user, I think it should be a system user without sudo, password, or shell; and that is a good way to invoke a system user with a shell. It would require more refactoring ipfspodcasting-install.sh and IPFS-Node-Installation.sh than I can do at this moment.

And if you are assuming running as root, the sudo would not be necessary."

I'll respond to the last point first. I didn't use "sudo" to invoke the user change. That's a change that @Cameron did to my code since he wasn't running from root on a VPS or VM instance. :-D All the "sudo" entries you came across weren't from me and won't be in the final revision.

The "ipfs" user with a password and sudo privileges is what's needed to run things properly from a non-root account and easy IPFS setup using the "ipfspodcasting-install.sh" which I do not modify one bit. It's used as is provided by @Cameron. I went down this path of contributing because running IPFS daemon, in this case, is not a sound security practice from a system admin's point of view.

The proof that the installation wrapper I've contributed works, which makes use of "ipfspodcasting-install.sh", is that I've spun up numerous VPS instances and have a fully functioning IPFS node in about 10-15 minutes time.

In reality, once the setup is complete and verified to be online, @Cameron's node management script handles the rest. There's no additional "refactoring" to do. :-) The 7 nodes I have are running smoothly.


suorcd commented "There are definitely some more jarring interface choices, but most of the new installs will most likely be on 22.04"

I beg to differ. A VPS provider, which is the intended deployment platform, offer versions of Ubuntu all the way back to 16.04 LTS.

And lastly, people are free to choose their platform of choice to deploy IPFS. However, even @Cameron's "RunNode" page (https://ipfspodcasting.com/RunNode/Hosted) clearly states Ubuntu and Debian. :-)

The show must go on! There are many more IPFS Podcasting nodes waiting to be spun properly.

Thank you for your contributions and discussion points!

Cameron-IPFSPodcasting commented 2 years ago

Thanks for the discussion. I'm still reading all the comments and will work on integrating everything (hopefully to everyone's satisfaction).

The added "sudo's" was my bad, and have been removed.

On garbage collection. It's not managed by the client script (yet). I'm planning to send GC commands in the request/response when I notice a node has reached it's usage limit (in the next client release). The client script will restart IPFS if it crashes (without --enable-gc). I don't think --enable-gc will be necessary on startup.

The only config change to IPFS is on the Umbrel where I add SwarmRelayClient in case the Umbrel is behind a firewall. Could add it to this client for consistency, but running your own server (more advanced) shouldn't need it.

CaffeinatedDNB commented 2 years ago

@Cameron-IPFSPodcasting

No problem. Sounds like a plan to me! :sunglasses:

As for the garbage collection, I thought you had mentioned in an earlier discussion that you would do garbage collection via "ipfs repo gc" once a week. Then again, I could be wrong. :laughing: In any event, the "--enable-gc" function shouldn't be necessary at startup as you mentioned.

As for those running your config using Umbrel, I have zero knowledge of that framework so please feel free to fork my install wrapper and included documentation to tweak it as needed for setup using Umbrel.

Looking forward to the updated script going live! :+1:

Thanks!

Cameron-IPFSPodcasting commented 2 years ago

Sorry, I didn't get to updating this today. Had other issues to take care of.

I had thought garbage collection ran weekly by default (if StorageMax was reached). I think I was looking at an old config. It looks like it never runs(!), which is more incentive to get that feature working in the next release.

Umbrel is a self-contained docker image that only needs to run ipfs init when the user enables the app. Everything else is already setup.

CaffeinatedDNB commented 2 years ago

No need to apologize. I know how that is all too well! We do what we can when we can. :+1:

Thanks for the update.

Regarding weekly garbage collection, perhaps you can add cron job to that end?

If you prefer to put a finer point on it, run the GC function when the system is usually idle (after collecting some system data.)

Or, put it on the admin by adding a cron job and asking them to choose a preferred time and day to run in your documentation.

As for the Umbrel setup, that sounds good. The scripts I contributed can possibly assist, after some modification (e.g. like removing the addition of the ipfs.service file, etc,) to secure the system by installing fail2ban, etc.

CaffeinatedDNB commented 2 years ago

@Cameron-IPFSPodcasting

Keeping busy huh? Or is that an understatement? 🤔😆

When you have a moment, please upload the latest version of the installation scripts and documentation from my recent post last week as it includes some of the suggestions made by "suorcd".

That way anyone that downloads it has the latest and greatest.

Thanks!

Cameron-IPFSPodcasting commented 2 years ago

Updated to the new file from @CaffeinatedDNB which includes a few of the changes from @suorcd.

Closing this PR. Thanks for the updates.