Charm Needed: Steam

Bug #885444 reported by Clint Byrum
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Juju Charms Collection
In Progress
Medium
Marco Ceppi

Bug Description

Steam game servers

Revision history for this message
Marco Ceppi (marcoceppi) wrote :

Pushed initial branch

tags: added: new-charm
removed: charm-needed
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Hi Marco! Here's an initial somewhat nit-picky review:

**********************

These things must be fixed before its good enough for the "official" charm store:

* Does not pass 'charm proof' without errors:

$ charm proof ./steam
E: no copyright file
W: all charms should provide at least one thing
E: missing apparmor directory (https://juju.ubuntu.com/AppArmor)

* There is no cryptographic verification of wget "http://storefront.steampowered.com/download/hldsupdatetool.bin". So it could be trojaned. Suggest storing the sha1sum of the file in the charm and verifying it (if the upstream file changes this would require updating the charm, so it would be even better if the upstream site provided an md5 or sha1 of the file somewhere).

**************************

These are other things that you should fix:

* The description states that Team Fortress 2 will be installed, but this is just the default. Should mention that any other steam server can be installed.

* config.yaml: mispelled "server" as "sever" description: "Region of the sever: -1 "

* also for this region, I'm curious why numbers are in use instead of strings.. is this a well known convention for all steam servers?

* from README "Steam is a charmed designed to deploy all Steam..." probably meant 'Steam is a charm'

* README, mispelled 'servers' as 'severs'

* hooks/install - not using set -e means that you must check the return code of each command to detect errors. suggest adding -e to the set command at the top. If that is not desired, suggest checking the outcome of the wget.

* hooks/install cp commands are not idempotent without -f argument, so if install is run again, those may fail.

* hooks/config-changed: same need for -e .. otherwise this hook is really quite nice!

* PROTIP: you can do this:

        JUJU_CFG_NAME=`echo "$CONFIG_NAME" | sed 's/STEAM_//'`

like this and not shell out 2 extra commands:

        JUJU_CFG_NAME=${CONFIG_NAME#STEAM_}

* Also you actually didn't use any bashisms that I see.. so /bin/sh would I think be acceptable, and actually run a bit faster (we're talking a tenth of a percent or maybe even *TWO* tenths of a percent! ;) since it would use dash.

Revision history for this message
Kapil Thangavelu (hazmat) wrote : Re: [Bug 885444] Re: Charm Needed: Steam

Excerpts from Clint Byrum's message of Mon Nov 07 22:06:50 UTC 2011:
>
> These things must be fixed before its good enough for the "official"
> charm store:
>
> * Does not pass 'charm proof' without errors:
>
> W: all charms should provide at least one thing
> E: missing apparmor directory (https://juju.ubuntu.com/AppArmor)

This shouldn't be required. My understanding is that this is an optional
thing, ie. We're not requiring charm authors to implement app armor, and having
them create an empty directory smells like a dead chicken.

cheers,
-kapil

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Agreed Kapil, I've removed the apparmor stuff from charm tools. So please disregard any warnings or errors you may have seen about those bits.

Revision history for this message
Marco Ceppi (marcoceppi) wrote :

Clint,

Thank you for the initial review. This charm is still undergoing a lot of mutations to make sure it works for all the steam games. My appologies for the apparmor absence, Git doesn't track empty directories by default! I've re-added it and implemented your feedback!

I look forward to having a more mature version of the charm available this week! Thanks :D

Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Excerpts from Marco Ceppi's message of Tue Nov 08 22:12:23 UTC 2011:
> Clint,
>
> Thank you for the initial review. This charm is still undergoing a lot
> of mutations to make sure it works for all the steam games. My
> appologies for the apparmor absence, Git doesn't track empty directories
> by default! I've re-added it and implemented your feedback!
>
> I look forward to having a more mature version of the charm available
> this week! Thanks :D

Cool Marco. I'll remove the 'new-charm' tag from the bug.. add it back in
when you're ready for another review.

Note that the AppArmor stuff is gone.. update your charm-tools from the PPA
and you won't get any warnings or errors about the apparmor dir anymore.

tags: removed: new-charm
Revision history for this message
Marco Ceppi (marcoceppi) wrote :

Ah, I didn't realize the new-charm tag was for charms that were ready for review.

Jorge Castro (jorge)
Changed in charms:
importance: Undecided → Medium
Revision history for this message
Charles Butler (lazypower) wrote :

As an additional comment, steam recently released SteamCMD - which is prime for running gaming servers.

https://developer.valvesoftware.com/wiki/SteamCMD

and as an example, here's a quick writeup I did of building a Starbound Server using SteamCMD - http://blog.dasroot.net/fabric-qemu-and-steamcmd/

Warning - if the game requires authentication to download, you will need to pull some juju magic out of a hat to get the 2fa working properly.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.