Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Opensuse support #7257
Conversation
|
!!build!! |
marcmolla
changed the base branch from
staging
to
develop
Apr 23, 2017
|
Changed to |
|
juju/utils PR is changed to juju/utils#277 |
|
!!build!! |
|
gofmt is sad: cmd/jujud/upgrade_mongo.go :-) |
|
sorry to hear that :) I updated those files with "go fmt " |
|
@marcmolla using go 1.8? |
|
yes, 1.8: $ go version
go version go1.8 linux/amd64Maybe I didn't expressed myself well before: I sent a couple of commits with the result of 'go fmt' over those three files. Hope that are ok now... |
|
!!build!! |
|
Do you need to update dependencies.tsv or ? checking: go build ... github.com/juju/juju/service/systemdservice/systemd/conf.go:49: undefined: "github.com/juju/utils/os".OpenSUSE github.com/juju/juju/cloudconfig/cloudinitcloudconfig/cloudinit/interface.go:430: undefined: "github.com/juju/utils/os".OpenSUSE github.com/juju/juju/worker/uniter/runner/contextworker/uniter/runner/context/env.go:23: undefined: "github.com/juju/utils/os".OpenSUSE github.com/juju/juju/testingtesting/base.go:253: undefined: "github.com/juju/utils/os".OpenSUSE |
|
Yes, it depends on the PR juju/utils#277, which is merged since yesterday. Let me check how to update dependencies.tsv for including this merge and I will came back... |
|
That actually went into juju master. You may just need to rebase. |
|
Yes, thanks for the advice! I didn't sync with master juju/utils after the PR. Now it should work, I build locally juju using a clone of the master of juju/utils. Also tests are working. |
|
Build looks good! Now you just need a proper review to land. |
howbazaar
requested changes
Apr 25, 2017
Over all this looks very good. I'm just wondering about the duplication in the cloudinit code.
| + | ||
| + if newMirror := cfg.PackageMirror(); newMirror != "" { | ||
| + cmds = append(cmds, LogProgressCmd("Changing package mirror does not yet work on OpenSUSE")) | ||
| + // TODO(bogdanteleaga, aznashwan): This should work after a further PR |
howbazaar
Apr 25, 2017
Owner
Do these TODO statements still make sense? If they are for common code, should the code be shared between openSUSE and CentOS?
marcmolla
Apr 27, 2017
Contributor
Yes, you are right. I can create a generic file with the common code and leave in the per OS source code the parts that differs...
I will send an update (probably tomorrow eob)
thanks!
| @@ -68,6 +68,8 @@ var ( | ||
| // CentOSGroups is the set of unix groups to add the "ubuntu" user to | ||
| // when initializing a CentOS system. | ||
| CentOSGroups = []string{"adm", "systemd-journal", "wheel"} | ||
| + |
|
@marcmolla I presume you built the openSUSE image by following what's done in https://github.com/axw/juju-lxd-centos-image-builder? I've extracted the common bits into a separate package and command, https://github.com/axw/lxdimage, along with an example which builds an openSUSE Leap image according to your instructions. |
|
@axw I just realized that you are the author of the blog that put me in the right direction :) Yes, basically I did manually what your script do automatically. I have in my TODOlist to automatize the process... I will take a look to your update! thanks |
|
@howbazaar I refactor this part but I still have to check it, as is not working well in my environment. I'll continue next week (we have some day off due to May 1st) |
|
@howbazaar sorry by the delay, I was on a short vacations. Now the update is working... I tried to reuse as much as possible the CentOS code and I created a new interface for the "delta" in the code. Hope now is fine Thanks! |
axw
approved these changes
May 5, 2017
this looks great, thank you @marcmolla !
just a few small comments, then I think it's good to land. please let me know when you're ready for it to be merged.
| + return []string{ | ||
| + "curl", | ||
| + "bridge-utils", | ||
| + //"cloud-utils", Put as a requirement to the cloud image (requires subscription) |
axw
May 5, 2017
Member
are you saying that only users with SLES subscriptions can get cloud-init? if not, what needs to be done? cloud-init will be required for most providers to work
marcmolla
May 7, 2017
Contributor
No, cloud-init package is in the public repo. Other packages (like cloud-utils) should be in the cloud repo.
For public clouds (I checked aws), you can deploy openSUSE with access to that repo (the prices of this option is ten times greater than deploying ubuntu, so I guess that you are paying the fee/royalty in the instance cost)
For privates clouds I think that you need access to cloud repo. I will check this in my next step: to use OpenStack as provider.
| + "systemctl is-active firewalld &> /dev/null && systemctl stop firewalld || true", | ||
| + `sed -i "s/^.*requiretty/#Defaults requiretty/" /etc/sudoers`, | ||
| + //Scripts assume ubuntu group for ubuntu user... | ||
| + `(grep ubuntu /etc/group) || groupadd ubuntu`, |
axw
May 5, 2017
Member
shouldn't cloud-init be creating the ubuntu user with the ubuntu group? maybe we need to add "ubuntu" to OpenSUSEGroups?
marcmolla
May 7, 2017
Contributor
By default new users are created using "users" group. "ubuntu" group does not exists and adding it to OpenSUSEGroups has no effect (group is not created...). For this reason I added these two lines here.
axw
May 8, 2017
Member
OK. cloud-init does support adding new groups, but we don't expose or use it in Juju at the moment. I may change this later to do it through cloud-init -- this is fine for now.
| @@ -613,8 +613,11 @@ func (u *UpgradeMongoCommand) removeOldDb(dataDir string) error { | ||
| func satisfyPrerequisites(operatingsystem string) error { | ||
| // CentOS is not currently supported by our mongo package. | ||
| + // I guess that is the same for Opensuse |
| @@ -78,9 +78,11 @@ func InitUbuntuUser(host, login, authorizedKeys string, read io.Reader, write io | ||
| return nil | ||
| } | ||
| +//For OpenSUSE we need to create ubuntu group and associate it to ubuntu user |
| @@ -192,6 +194,9 @@ os_id=$(grep '^ID=' /etc/os-release | tr -d '"' | cut -d= -f2) | ||
| if [ "$os_id" = 'centos' ]; then | ||
| os_version=$(grep '^VERSION_ID=' /etc/os-release | tr -d '"' | cut -d= -f2) | ||
| echo "centos$os_version" | ||
| +elif [ "$os_id" = 'opensuse' ]; then | ||
| + os_version=$(grep '^VERSION_ID=' /etc/os-release | tr -d '"' | cut -d= -f2) | ||
| + echo "opensuseleap" |
axw
May 5, 2017
Member
should we be checking for a specific version of openSUSE before claiming it's Leap?
|
I did a little bit of experimentation, and found that we can almost start an openSUSE VM in Azure with the diff below applied. I believe the reason why it's not quite enough is due to the way the software updates are executed. I removed the "--quiet" from zypper commands to see what was going on, and the last line in /var/log/cloud-init-output.log is:
Because the openSUSE image does not come with cloud-init pre-installed, my diff adds a "VM extension" to the Azure VM, which will be run by the Azure agent. I suspect what's happening is the package update is updating the Azure agent, which is killing the running process and interrupting the process. So almost, but not quite. You probably don't care if you're doing VNF things anyway, but someone else might. diff --git a/provider/azure/environ.go b/provider/azure/environ.go
index 3e44f96..bb2fea7 100644
--- a/provider/azure/environ.go
+++ b/provider/azure/environ.go
@@ -727,10 +727,10 @@ func (env *azureEnviron) createVirtualMachine(
DependsOn: vmDependsOn,
})
- // On Windows and CentOS, we must add the CustomScript VM
- // extension to run the CustomData script.
+ // On Windows, CentOS, and OpenSUSE, we must add the CustomScript
+ // VM extension to run the CustomData script.
switch seriesOS {
- case os.Windows, os.CentOS:
+ case os.Windows, os.CentOS, os.OpenSUSE:
properties, err := vmExtensionProperties(seriesOS)
if err != nil {
return errors.Annotate(
@@ -958,7 +958,7 @@ func newOSProfile(
return nil, os.Unknown, errors.Trace(err)
}
switch seriesOS {
- case os.Ubuntu, os.CentOS:
+ case os.Ubuntu, os.CentOS, os.OpenSUSE:
// SSH keys are handled by custom data, but must also be
// specified in order to forego providing a password, and
// disable password authentication.
diff --git a/provider/azure/internal/imageutils/images.go b/provider/azure/internal/imageutils/images.go
index dfaf8d6..b254ae5 100644
--- a/provider/azure/internal/imageutils/images.go
+++ b/provider/azure/internal/imageutils/images.go
@@ -30,6 +30,9 @@ const (
ubuntuPublisher = "Canonical"
ubuntuOffering = "UbuntuServer"
+ openSUSELeapOffering = "openSUSE-Leap"
+ susePublisher = "SUSE"
+
windowsServerPublisher = "MicrosoftWindowsServer"
windowsServerOffering = "WindowsServer"
@@ -96,8 +99,17 @@ func SeriesImage(
return nil, errors.NotSupportedf("deploying %s", series)
}
+ case os.OpenSUSE:
+ publisher = susePublisher
+ offering = openSUSELeapOffering
+ switch series {
+ case "opensuseleap":
+ sku = "42.2"
+ default:
+ return nil, errors.NotSupportedf("deploying %s", series)
+ }
+
default:
- // TODO(axw) CentOS
return nil, errors.NotSupportedf("deploying %s", seriesOS)
}
diff --git a/provider/azure/userdata.go b/provider/azure/userdata.go
index b1b179a..3251e14 100644
--- a/provider/azure/userdata.go
+++ b/provider/azure/userdata.go
@@ -19,7 +19,7 @@ func (AzureRenderer) Render(cfg cloudinit.CloudConfig, os jujuos.OSType) ([]byte
switch os {
case jujuos.Ubuntu:
return renderers.RenderYAML(cfg, utils.Gzip, renderers.ToBase64)
- case jujuos.CentOS:
+ case jujuos.CentOS, jujuos.OpenSUSE:
return renderers.RenderScript(cfg, renderers.ToBase64)
case jujuos.Windows:
return renderers.RenderYAML(cfg, renderers.WinEmbedInScript, renderers.ToBase64)
diff --git a/provider/azure/vmextension.go b/provider/azure/vmextension.go
index 5bfdff0..a67c7b9 100644
--- a/provider/azure/vmextension.go
+++ b/provider/azure/vmextension.go
@@ -42,7 +42,7 @@ func vmExtensionProperties(os jujuos.OSType) (*compute.VirtualMachineExtensionPr
extensionPublisher = windowsCustomScriptPublisher
extensionType = windowsCustomScriptType
extensionVersion = windowsCustomScriptVersion
- case jujuos.CentOS:
+ case jujuos.CentOS, jujuos.OpenSUSE:
commandToExecute = linuxExecuteCustomScriptCommand
extensionPublisher = linuxCustomScriptPublisher
extensionType = linuxCustomScriptType |
|
@axw, I updated the PR with your last comments and I am ready to merger it when you think that is ok... should I collapse my commits? In this case, I need an advise to do it correctly: should I squash my commits into the first one and pick the other contributors commits? thanks |
Yes, please squash them all into the first commit. There should not be any other commits if you have an up to date "develop" branch locally, and are rebasing off that. |
If I try to rebase following the doc: I have more than 500 commits with a lot of conflicts. I was able to squash all of them with the first one by I ended with 900 files changed (?!). I think I have the same problem than in juju/utils, not sure if I did something wrong or maybe doc is not updated (is always talking about 'master' as origin branch, not 'develop'). Any idea? |
You are correct, the docs are out of date. Please rebase against develop, not master. I'll update the docs - sorry, and thanks for pointing that out. |
|
I have the same problem. I think that the root of my issue is that I branch out from master and I pulled from there until I tried the PR, and now I have a mess in the commits (maybe this point is also outdated in the doc...) I am going to try a different approach (maybe to reset my branch and to create a new commit with only my changes.) |
Indeed, you should pull from develop, rebase on develop, and propose against develop. I've created #7318, which I think fixes each of these outdated instructions.
OK. Let me know if you'd like me to merge again, although if you plan to keep contributing (and I hope you do!), it would be best to work out what's going on. |
|
ok, seems that "git reset" is what I needed :). I'll check if build is ok (previous one failed in a windows test, not sure if it is something related to my PR) |
Thanks for rebasing. Let me know once you've confirmed it's ready and I'll merge it. I believe the Windows test failure is unrelated. |
added a commit
that referenced
this pull request
May 10, 2017
|
I think it is ready for merge. Build in Jenkins has errors:
but I'm not sure if it is related to the PR |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit 8adf3b6
into
juju:develop
May 10, 2017
1 check failed
|
Thanks again @marcmolla :) |
marcmolla
deleted the
marcmolla:opensuse_support
branch
May 10, 2017
|
Yes, my intention is to send a PR to the doc for including OpenSUSE and for documenting the manual provisioning and the containers support. Regarding the agent, do you know how it will be included in the official stream? or should OpenSUSE users generate their own agent-stream? |
Great, thank you. If you also blog about it, please let us know on the juju mailing list or IRC, so we can help spread the word.
I'll have to ask with the QA people. I think it's reasonable for the agents to be built along with the other distros and included in streams.canonical.com. |
marcmolla commentedApr 20, 2017
•
Edited 1 time
-
marcmolla
Apr 24, 2017
Description of change
This change add the support of OpenSUSE Leap (42 series) to Juju.
Main reason for the change is that most of our NVF run on top of OpenSUSE/SLES and I was doing some hands-on for evaluating Juju as a generic VNF-M. This PR (and other for juju/utils) is the result of my work.
With these PR, Juju users are able to deploy OpenSUSE charms in LXD and in manual provisioned OpenSUSE host.
This PR depends on: juju/utils#277
QA steps
A part of the testing that I added, it is possible to deploy an OpenSUSE charm:
https://github.com/marcmolla/juju-OpenSUSE/tree/master/charms/test-opensuseleap
using a modified OpenSUSE image:
https://github.com/marcmolla/juju-OpenSUSE/blob/master/lxd-opensuse-42.2-image.md
I also describe how I tested the software locally:
https://github.com/marcmolla/juju-OpenSUSE/blob/master/test-compiled-Juju.md
For all those testing we also need another PR to juju/utils (juju/utils#272)
Documentation changes
If this PR is accepted, we should include reference to OpenSUSE in general doc.
Bug reference
N/A