Skip to content

Conversation

greg-1-anderson
Copy link

When I tried to install minecraft with this module, I found that version 1.5.2 was being downloaded, and the minecraft server was not starting up cleanly. I made the following changes to make things work more smoothly:

  • Enabled service minecraft and installed sysv-rc-conf or chkconfig to insure that the minecraft would start.
  • Added a 'version' parameter to minecraft class.
  • Initialized server.properties from a template file, so that the server_prop class will work. (EDIT: Backed this out)
  • Update name of s3file to s3::file. (EDIT: Backed this out)

…ure the minecraft will start. Add a 'version' parameter to minecraft class. Initialize server.properties from a template file, so that the server_prop class will work. Update name of s3file to s3::file.
@branan
Copy link
Owner

branan commented Nov 7, 2013

Hi Greg,

As I mentioned in your s3file pull, I don't think that's a good change to be making.

I also have some concerns about managing the properties file as a template. If a module user manages a property that conflicts with this template, every puppet run will re-apply the template and then apply their properties again, which is not awesome and creates a lot of noise in reports. If the file needs to exist for compatibility with the file_line in certain versions of stdlib, I'd suggest using a file resource without a content attribute. If certain properties need to be set to make the server happy, they should be managed as individual server_prop resources in this module.

The service fixups and updated download location do look good, though. I'd be happy to merge those in.

@greg-1-anderson
Copy link
Author

Thank you for bearing with me as I work through my puppet issues.

I am running puppet inside of an Ubuntu 12.04 LTS virtualbox managed by vagrant -- see https://github.com/greg-1-anderson/minecraft-environment. (That project contains this one as a git subtree; I'll add a CONTRIBUTING.md to describe how to commit changes back to the original project, etc.). I picked the version of puppet that I did because it is what came pre-installed with 12.04 LTS. I'll find an easy way to upgrade puppet so that I don't have to fix antiquated problems. Maybe I should just target a newer version of Ubuntu, but part of my goal is to make it really easy to deploy to a live server, and 12.04 LTS is really common with hosting providers right now. One way or another, I'll figure something out, though.

I added the properties file as a template to work around a puppet complaint that the file line could not be updated if the file did not exist yet. I'm going to also assume that this problem will go away when I upgrade puppet.

I will submit a better pull request once these issues are resolved.

@branan
Copy link
Owner

branan commented Nov 7, 2013

If you want to track the latest Puppet Labs releases, you can simply enable our apt repository by downloading the appropriate package from http://apt.puppetlabs.com/ (puppetlabs-release-precise, in your case).

@greg-1-anderson
Copy link
Author

Thanks; I appreciate the extra pointers. My goal here is to learn minecraft modding so that I can teach kids how to do it. I therefore want to make vagrant up "just work". I'm not sure that I can do that without making my own box; I think that the easiest solution is to just use a more recent version of Ubuntu with vagrant. Since mc is in Java, I think it's pretty likely that whatever I put together on virtualbox with a recent Ubuntu should work on 12.04 LTS; I'll just have to include instructions on upgrading Puppet as you recommend prior to the first deployment.

@greg-1-anderson
Copy link
Author

The above commit brings back s3file, and removes the template file for server.properties. This should be good to go now.

Oh, and also, as an aside, http://puppet-vagrant-boxes.puppetlabs.com/ was very useful in providing a vagrant box for Ubuntu 12.04 that contained a 3.x version of Puppet pre-installed.

@branan
Copy link
Owner

branan commented Nov 8, 2013

This looks good to me now 👍. I'm gonna want to take some time and try it out before I merge it in, but I don't have any complaints looking over it.

I'm not a fan of pinning a version by default (mostly due to the potential for code rot), but for now pointing to 1.7.2 is fine. Longer-term, I'll probably see about writing a parser function based on the information in http://gaming.stackexchange.com/a/123443 that will give us the latest minecraft version to use as a default value of 'latest' for the version. For now I'm happy to get this merged, though.

I'm glad the vagrant boxes were helpful for you! I'll pass that on to the folks responsible for them :)

@greg-1-anderson
Copy link
Author

stdlib has a 'parsejson' function, so I got this far:

$ puppet apply -e '$f = file("versions.json") $v = parsejson($f) notify { $v["latest"]["release"]: }'

Notice: 1.7.2

Is it possible to use curl to assign the contents of a URL directly to a variable? If not, I suppose it would always be possible to create a temporary file, and then use an expression similar to the example above.

@greg-1-anderson
Copy link
Author

So, I naively tried this:

  exec { "fetch versions":
    path    => ['/bin', '/usr/bin', 'sbin', '/usr/sbin'],
    command => "curl -L -o ${homedir}/versions.json https://s3.amazonaws.com/Minecraft.Download/versions/versions.json",
  }

  $versions = parsejson(file("${homedir}/versions.json"))

  $minecraft_version = $version ? {
    latest => $versions["latest"]["release"],
    default => $version,
  }

  notify { "Minecraft version is $minecraft_version": }

  s3file { "${homedir}/minecraft_server.jar":
    source  => "Minecraft.Download/versions/$minecraft_version/minecraft_server.$minecraft_version.jar",
    require => User[$user],
  }

However, this left me scratching my head over the way for specifying that the "fetch versions" exec should happen before the parsejson(file()) does, as Puppet has no syntax for this. Seems variables are always evaluated first, so I guess there's no way to do this in standard Puppet syntax? I presume this is what you meant by "parser function" -- not a function that parses the json, but a new command. A version of "file" that read data from a URL would do the trick. The other question is, if "minecraft_server.jar" contains an older version of Minecraft, how do you force it to be downloaded again? I suppose we could change it to s3file { "${homedir}/minecraft_server_$minecraft_version.jar":, and then notify a rule to create a symbolic link from minecraft_server.jar to minecraft_server_$minecraft_version.jar.

I could do the later, but I think that writing a parser function is beyond me for now.

@greg-1-anderson
Copy link
Author

I should also add that I have one more open issue that you may also encounter. On the very first run, Minecraft starts up, but it immediately stops again as soon as it finishes creating the world. Running sudo /etc/init.d/minecraft start, or simply restarting the server both work just fine, and Minecraft stays up on the second run. I thought it might have to do with using openjdk instead of Oracle Java, but when I tried with Oracle Java, it behaved the same way. I currently have no theory as to why it's stopping on the first run.

I think that this problem is unrelated to this PR, though.

@greg-1-anderson
Copy link
Author

The commit above insures that minecraft will be downloaded again if the 'version' parameter changes. It does not support 'latest'.

@didlix
Copy link

didlix commented Sep 1, 2014

hmm. @branan @greg-1-anderson how is this progressing?

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.

3 participants