[vsphere] Add ability to specify local templates for vsphere #12894

gabriel-samfira · 2021-04-19T13:27:20Z

This PR adds ability to specify local templates via simplestreams using the image-id format. I

QA steps

Steps are as follows:

Identify the region you want to generate simplestreams for: juju regions yourcloud
Create a template for a series inside that particular datacenter. The template should have the same characteristics as the ones Juju creates if no simplestreams is supplied by the user. As a quick hack, simply deploy a controller, and clone the resulting template in a different location.
Generate simplestreams
Deploy a new controller using the generated simplestreams
Check that Juju skips downloading the template and moves straight to cloning the template.

Generate tools metadata

mkdir -p $HOME/simplestreams/tools/proposed
cp $GOPATH/bin/jujud $HOME/simplestreams/tools/proposed
cd $HOME/simplestreams/tools/proposed
tar czf juju-`./jujud version`.tgz ./jujud
rm ./jujud
cd ~

juju-metadata generate-agents -d $HOME/simplestreams --stream proposed

Generate image metadata

When generating simplestreams, use the relative path to the template in relation to the datacenter path (/datacenter-name/vm):

juju-metadata generate-image -d $HOME/simplestreams/ -i "gsamfira/juju-focal-template" -s focal -r datacenter6.5 -u 10.107.8.11

This will generate the following: https://paste.ubuntu.com/p/9zk5hX7wR6/

Given the above simplesrteams, the provider will look for the template in:

/datacenter6.5/vm/gsamfira/juju-focal-template

then deploy a new controller:

juju bootstrap --metadata-source $HOME/simplestreams --debug --verbose --show-log  vsphere vsphere-controller

Bug reference

https://bugs.launchpad.net/juju/+bug/1867154

hmlanigan · 2021-04-26T13:38:59Z

provider/vsphere/internal/vsphereclient/client.go

+}
+
+func (c *Client) ListVMTemplates(ctx context.Context, path string) ([]*object.VirtualMachine, error) {
+	c.logger.Debugf("ListVMTemplates() path=%q", path)


Let's keep this log message at a Trace level

gabriel-samfira · 2021-04-26T19:10:09Z

This PR still needs some new tests, but it should be ready for review.

hmlanigan · 2021-04-28T17:26:43Z

provider/vsphere/vm_template.go

+	seriesTemplatesFolder := v.seriesTemplateFolder(series)
+	seriesTemplates, err := v.env.client.ListVMTemplates(ctx, path.Join(seriesTemplatesFolder, "*"))
+	if err != nil {
+		logger.Errorf("failed to fetch templates: %v", err)


This causes misleading error messages during bootstrap, when the folder for the bootstrapping controller hasn't been created yet and not simplestreams supplied

... Launching controller instance(s) on boston-vsphere/Boston... ERROR failed to fetch templates: path not found: juju-ci-root/Juju Controller (baf1f6a2-f6d7-44f1-8916-7165113f7f8f)/templates/focal/* not found - juju-dc6839-0 (arch=amd64 mem=3.5G) f81b245c98d18fec1564e6b374c346f1ef14394c9e836ee77e1e58754242a4f9" Installing Juju agent on bootstrap instance ...

hmlanigan · 2021-04-28T18:24:40Z

During QA:

Bootstrap used the simplestreams defined template, but juju deploy ubuntu --series focal did not. A new template was added to the "Juju Controller (<uuid)/templates/focal) folder, which was not created beforehand. The simplestreams template is for focal: https://paste.ubuntu.com/p/VJsZTtVnV5/

hmlanigan

The one issue found during QA that needs to be resolved, as well as items listed below.

provider/vsphere/internal/vsphereclient/client.go

hmlanigan · 2021-04-28T19:00:10Z

provider/vsphere/internal/vsphereclient/client.go

+	items, err := finder.VirtualMachineList(ctx, path)
+	if err != nil {
+		if _, ok := err.(*find.NotFoundError); ok {
+			return nil, errors.NotFoundf("path not found: %s", path)


Remove "not found" from error message, redundant as errors.NotFoundf() prints it at the end. Perhaps use errors.Annotatef instead?

hmlanigan · 2021-04-28T19:33:05Z

provider/vsphere/internal/vsphereclient/createvm.go

+
+type ImportOVAParameters struct {
+	StatusUpdateParams StatusUpdateParams
+	// ReadOVA returns the location of, and an io.ReadCloser for,


nit: new line above for consistency.

hmlanigan · 2021-04-28T19:34:00Z

provider/vsphere/internal/vsphereclient/createvm.go

+	Clock clock.Clock
+}
+
+type ImportOVAParameters struct {


Comment header for struct

hmlanigan · 2021-04-28T20:47:06Z

provider/vsphere/vm_template.go

+			return arch, nil
+		}
+	}
+	return "", errors.NotFoundf("arch tag not found for VM")


"arch tag for VM" is clearer with ": not found" tacked to the end when the error is a string. otherwise error string is: "arch tag not found for VM: not found"

hmlanigan · 2021-04-28T20:51:02Z

provider/vsphere/vm_template.go

+	for _, item := range desiredArches {
+		if item == arch {
+			return true
+		}
+	}
+	return false


return set.NewStrings(desiredArches...).Contains(arch)

hmlanigan · 2021-04-28T20:52:09Z

provider/vsphere/vm_template.go

+	}
+	logger.Debugf("Series templates: %v", seriesTemplates)
+	if len(seriesTemplates) == 0 {
+		return nil, "", errors.NotFoundf("no valid templates found")


"valid templates"

hmlanigan · 2021-04-28T20:52:24Z

provider/vsphere/vm_template.go

+			break
+		}
+		if vmTpl == nil {
+			return nil, "", errors.NotFoundf("no valid templates found")


"valid templates"

hmlanigan · 2021-04-28T20:56:33Z

provider/vsphere/vm_template.go

+// vmTemplateManager implements a template registry that
+// can return a proper VMware template given a series and
+// image metadata.
+type vmTemplateManager struct {


Where are the unit tests for the vmTemplateManager?

gabriel-samfira · 2021-05-03T12:49:41Z

During QA:

Bootstrap used the simplestreams defined template, but juju deploy ubuntu --series focal did not. A new template was added to the "Juju Controller (<uuid)/templates/focal) folder, which was not created beforehand. The simplestreams template is for focal: https://paste.ubuntu.com/p/VJsZTtVnV5/

This is probably due to the fact that during bootstrap, juju grabbed the upstream jujud binary, which does not have the changes.

Make sure you also generate the agent metadata using:

juju-metadata generate-agents --stream proposed

This will look for packaged binaries in $HOME/.local/share/juju/tools/proposed/ and build appropriate metadata for whatever it finds in there. I will update the PR description.

When generating simplestreams, use a relative path to the template instead of just the template name. The relative path is applied to the datacenter path when searching for the template.

Windows can now be supported with the adition of local templates. The template must conform to the following: * A supported version of Windows must be installed inside * vApp config settings must have the proper ovfenv (same ones as Ubuntu - instance-id, userdata, etc) * VMware tools must be installed * Cloudbase-Init must be installed, and the cloudbaseinit.metadata.services.ovfservice.OvfService set as the only value in the metadata_services config option in both cloudbase-init.conf and cloudbase-init-unattend.conf. * Sysprep the instance using: sysprep.exe /generalize /oobe /unattend:C:\Program Files\Cloudbase Solutions\Cloudbase-Init\Conf\unattend.xml /shutdown (copy that file in C:\Unattend.xml if you encouner errors) * before cloning as template, network adapter must be removed, and any ISO file attached to the CDROM must be detached.

hmlanigan · 2021-05-04T19:26:16Z

When using --build-agents, should need the juju metadata piece. Redid the QA, as expected: when deploying ubuntu with focal, no new template was created, when deploying ubuntu with bionic, a new template was created.

hmlanigan

A few small items to resolve, then I will approve and merge.

hmlanigan · 2021-05-04T19:32:23Z

provider/vsphere/export_test.go

+	"github.com/juju/juju/environs"
+	"github.com/juju/juju/environs/imagemetadata"
+	"github.com/juju/juju/provider/vsphere/internal/vsphereclient"
+	"github.com/vmware/govmomi/object"
+	"github.com/vmware/govmomi/vim25/types"


should be 2 stanza of imports

provider/vsphere/userdata_test.go

provider/vsphere/vm_template.go

provider/vsphere/vm_template_test.go

hmlanigan

LGTM now, all testing is good.

hmlanigan · 2021-05-05T14:30:03Z

!!build!!

hmlanigan · 2021-05-05T14:56:10Z

$$merge$$

#12963 Merge most recent 2.9 commits (up to commit d597250 for #12948) : #12892 Add CLA checker GH action #12937 Actions with errors queuing them can be listed #12940 Fix ssh to sidecar charm container #12941 Fix/lp 1925735 #12943 Ensure upgraded 2.8 models can start containers on machines #12947 Remove redundant AddressConfigMethod #12949 Remove redundant AddressConfigMethod #12950 Fix non-linux network config source #12894 Add ability to specify local templates for vsphere #12948 Select default charm url depending on controller version Conflicts were due to actions api changes from #12937 ``` # apiserver/facades/agent/uniter/uniter_test.go # apiserver/facades/client/action/action_test.go # apiserver/facades/client/action/legacy_test.go # apiserver/facades/client/action/operation.go # apiserver/params/internal.go # cmd/juju/application/deploy_test.go # go.mod # go.sum # scripts/win-installer/setup.iss # snap/snapcraft.yaml # state/action.go # state/action_test.go # state/allwatcher_internal_test.go # state/cleanup_test.go # state/interface.go # state/machine.go # state/machine_test.go # state/migration_export_test.go # state/migration_import_test.go # state/operation_test.go # state/state_test.go # state/unit.go # state/unit_test.go # state/watcher_test.go # version/version.go # worker/uniter/runner/context/context_test.go # worker/uniter/runner/context/contextfactory_test.go # worker/uniter/runner/factory_test.go # worker/uniter/util_test.go ``` ## QA steps See PRs

hpidcock added the 2.9 label Apr 20, 2021

hmlanigan reviewed Apr 26, 2021

View reviewed changes

gabriel-samfira marked this pull request as ready for review April 26, 2021 19:06

gabriel-samfira force-pushed the add-custom-simplestreams branch from a497b6e to 440bf3c Compare April 26, 2021 20:06

hmlanigan reviewed Apr 28, 2021

View reviewed changes

hmlanigan requested changes Apr 28, 2021

View reviewed changes

gabriel-samfira added 11 commits May 3, 2021 17:36

Add ability to specify local templates for vsphere

e42d559

Use relative path to find template

65fa5a6

When generating simplestreams, use a relative path to the template instead of just the template name. The relative path is applied to the datacenter path when searching for the template.

Remove unused fields and function

d06391c

Fix internal package tests

f1612ee

Fix linting warnings

a98f8de

Fix downloadAndImportTemplate when vmfolder

7c1e810

Fix remaining tests

99e34a8

Added copyright header and fixed mock

fc73613

Add vmTemplateManager tests

f9db17b

Amended comments, fixed mock

44df238

Added header to export_test.go

0ec66de

gabriel-samfira force-pushed the add-custom-simplestreams branch from 0fdb113 to 0ec66de Compare May 3, 2021 14:46

gabriel-samfira added 2 commits May 3, 2021 18:07

Add test for ListVMTemplates

06d3222

gabriel-samfira force-pushed the add-custom-simplestreams branch from 37e263f to ccf4222 Compare May 4, 2021 11:02

hmlanigan reviewed May 4, 2021

View reviewed changes

Fix imports aand move test

01bc549

hmlanigan approved these changes May 5, 2021

View reviewed changes

jujubot merged commit aaeeacd into juju:2.9 May 5, 2021

wallyworld mentioned this pull request May 12, 2021

Merge 2.9 #12963

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[vsphere] Add ability to specify local templates for vsphere #12894

[vsphere] Add ability to specify local templates for vsphere #12894

gabriel-samfira commented Apr 19, 2021 •

edited

Loading

hmlanigan Apr 26, 2021

gabriel-samfira commented Apr 26, 2021

hmlanigan Apr 28, 2021 •

edited

Loading

hmlanigan commented Apr 28, 2021

hmlanigan left a comment

hmlanigan Apr 28, 2021

hmlanigan Apr 28, 2021

hmlanigan Apr 28, 2021

hmlanigan Apr 28, 2021

hmlanigan Apr 28, 2021

hmlanigan Apr 28, 2021

hmlanigan Apr 28, 2021

hmlanigan Apr 28, 2021

gabriel-samfira commented May 3, 2021

hmlanigan commented May 4, 2021

hmlanigan left a comment

hmlanigan May 4, 2021

hmlanigan left a comment

hmlanigan commented May 5, 2021

hmlanigan commented May 5, 2021

[vsphere] Add ability to specify local templates for vsphere #12894

[vsphere] Add ability to specify local templates for vsphere #12894

Conversation

gabriel-samfira commented Apr 19, 2021 • edited Loading

QA steps

Generate tools metadata

Generate image metadata

Bug reference

Choose a reason for hiding this comment

gabriel-samfira commented Apr 26, 2021

hmlanigan Apr 28, 2021 • edited Loading

Choose a reason for hiding this comment

hmlanigan commented Apr 28, 2021

hmlanigan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabriel-samfira commented May 3, 2021

hmlanigan commented May 4, 2021

hmlanigan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hmlanigan left a comment

Choose a reason for hiding this comment

hmlanigan commented May 5, 2021

hmlanigan commented May 5, 2021

gabriel-samfira commented Apr 19, 2021 •

edited

Loading

hmlanigan Apr 28, 2021 •

edited

Loading