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

Future support of this project #105

Open
ddelnano opened this issue Dec 28, 2020 · 11 comments
Open

Future support of this project #105

ddelnano opened this issue Dec 28, 2020 · 11 comments

Comments

@ddelnano
Copy link

ddelnano commented Dec 28, 2020

I've been recently sponsored to work on a terraform provider that integrates with Xenorchestra (and XCP-ng / xenserver). Part of the vision for that work is also strengthening other complementary tools (packer being one of them).

I've started a fork (ddelnano/packer-builder-xenserver) which is now fully functional. It improves upon this project with the following:

  1. Compiling the project on a recent go version and removed the closed source / missing dependencies.
  2. Packer version upgraded to the latest version (v1.6.5)
  3. Updated the boot command to use the xenserver console api (as mentioned here). This fixed an issue related to an upstream qemu migration (details on XSO-906)

Is there any interest in the xenserver team maintaining this project? If not, I would like to take over ownership of it. In order to transfer ownership I'm suggesting the following changes:

  1. Update this project's README stating that it is no longer supported and that my fork (ddelnano/packer-builder-xenserver) is currently supported
  2. Update Packer's official documentation to point to my fork.

If you don't have any issues with what I'm suggesting above, please let me know and I'd be happy to make the PRs myself.

@fanuelsen
Copy link

I would suggest that the xenserver team makes this happen.

@ddelnano
Copy link
Author

The project now supports the latest packer sdk and will allow being installed by packer init (once packer 1.7.1 is released) so it is still receiving constant attention.

@kirannhegde
Copy link

Hello @ddelnano Is there a way we could merge your changes into this?

@kirannhegde
Copy link

@ddelnano Ideally, I would want to merge your changes first to my fork of this repository and then merge the changes back to this repository. The reason being, my fork contains a lot more changes and bug fixes than in the official repository.

@ddelnano
Copy link
Author

@kirannhegde apologies for the delay. I've been away since your previous messages.

I think merging things in that order will be difficult. My changes were pretty significant and I ditched large portions of the code base to get it working:

  • Using a different Xenserver (citrix hypervisor) client
  • Using the latest packer SDK
  • Using latest go, using go modules, goreleaser as a build system and automatically publishes releases to github
  • Updated examples

I honestly think it would be easier to identify what pieces of your fork are important and port them to my branch. Is it even possible to build upstream or your fork? I wasn't able to due the old go version and missing dependencies. As a result, I think trying to bring upstream or your fork up to date is going to be a more significant effort but I'm open to ideas.

I'll take a look at what your fork has included and see if I can get an idea of how much work it would be to port those changes.

@ddelnano
Copy link
Author

I've done a very quick assessment of what your fork adds over upstream and labeled what I think my fork is missing here.

I'll clean it up to be easier to read, but a summary of the items are listed below:

  • Your fork has many changes related to socat. The ddelnano fork doesn't use socat since it uses the VM.get_consoles and VM.get_location APIs to access VNC (details and code reference). I think it's possible these changes might not be necessary to port.
  • Your fork has changes related to winrm. winrm is something I haven't verified if it works so there is definitely work that needs to be done here. I don't have experience with Windows so this is what I would view as most difficult.
  • XVA builder for ddelnano fork is not tested and is missing improvements your fork has
  • More minor functionality improvements in your fork (indicated in the spreadsheet above)

I still think adding the missing parts to the ddelnano fork would be easier after reviewing the above. But let me know what you think.

@kirannhegde
Copy link

Hello @ddelnano Thank you for the list. I am planning to go over the list today and see what would be the best way forward.I am new to GoLang myself but i consider this as a good opportunity to get my hands dirty.

@ddelnano
Copy link
Author

@kirannhegde did you get a chance to review the list?

@kirannhegde
Copy link

Hello @ddelnano Not yet unfortunately. I am right now fixing an issue in my fork where the absence of the tools iso in XS 8.2 is causing the Packer run to fail. I will definitely do through your list once i am done with my code change.. To give you some perspective of my fork, my fork contains the following changes:
1)Forked from the official xenserver packer plugin repository
2)Changes from https://github.com/ctlam/packer-builder-xenserver/commits/master were merged into my fork.
3)On top of this, some minor fixes were made.

I will go through the changes once i have some time. I am also relatively new to GoLang, hence that is another thing that is kind of slowing the review process(I dont know the code flow of this plugin, i have just manually inspected it in the past). Hence, i will have to step through the code once to get a better understanding of the code flow.

@Zhengchai
Copy link
Contributor

Thanks for this contribution. this is good stuff.

@RodyKossen
Copy link
Collaborator

Hi @ddelnano, can we get in contact on merging your changes into our repository?

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

No branches or pull requests

5 participants