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.

Sign up for GitHub

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

Jump to bottom

Fix windows userdata #12859

Merged
merged 3 commits into from
May 20, 2021
Merged

Fix windows userdata #12859

merged 3 commits into from
May 20, 2021

Conversation

gabriel-samfira
Copy link
Contributor

@gabriel-samfira gabriel-samfira commented Apr 12, 2021 •
edited
Loading

This fixes an embarrassing bug of my own making, introduced in commit: 942456e

where I omitted to remove the corresponding parameters to fmt.Sprintf after modifying the string it was formatting.

I added a test for InstallCommands() as well as fixing some tests for password change that seem to have not been caught by the CI.

Update:

Given that unit agents seem to be gone now, and there is only one service being created on a machine, the machine agent is now run under the local jujud user on windows again.

A followup PR will be created at a later date that will remove code that was used to maintain unit agents on Windows.

QA steps

juju bootstrap
juju add-machine --series win2019

Wait for machine to boot.

juju run --machine 0 "sc.exe qc jujud-machine-0"

Check that SERVICE_START_NAME is .\jujud.

@hmlanigan hmlanigan self-requested a review April 12, 2021 19:58
@hmlanigan
Copy link
Member

check-multi-juju, when running the unit tests on ubuntu, found:
FAIL: userdatacfg_test.go:1387: cloudinitSuite.TestWindowsCloudInit
Here's a snippet of the failure: https://pastebin.ubuntu.com/p/XnTyGfgZ3g/

@hpidcock hpidcock added the 2.9 label Apr 18, 2021
@gabriel-samfira
Use jujud user for machine agent
abee5b2 
Since juju now only creates one service, the machine agent, we need
to run that as a normal user.
@hmlanigan
Copy link
Member

!!build!!

@hmlanigan
Copy link
Member

@gabriel-samfira please add QA steps to the PR description.

@hmlanigan
Copy link
Member

@gabriel-samfira,

During QA with 2.9, the windows machine gets stuck in pending. I'm using OpenStack and the Windows 2012 R2 eval KVM image you pointed me at. The machine is active, but the juju agent never connects to the controller.

I'm I building the windows agent binaries with:
$ GOOS=windows make install

I can successfully add a Windows 2012 R2 eval machine to a 2.8.10 controller on the same OpenStack.

@gabriel-samfira
Copy link
Contributor Author

@gabriel-samfira,

During QA with 2.9, the windows machine gets stuck in pending. I'm using OpenStack and the Windows 2012 R2 eval KVM image you pointed me at. The machine is active, but the juju agent never connects to the controller.

I'm I building the windows agent binaries with:
$ GOOS=windows make install

I can successfully add a Windows 2012 R2 eval machine to a 2.8.10 controller on the same OpenStack.

Try the following:

make release-install
GOOS=windows make release-install

Generate simplestreams for agents:

mkdir -p $HOME/simplestreams/tools/proposed
cp $GOPATH/bin/jujud $HOME/simplestreams/tools/proposed
cp $GOPATH/bin/windows_amd64/jujud.exe $HOME/simplestreams/tools/proposed
cd $HOME/simplestreams/tools/proposed
tar czf juju-2.9.1-ubuntu-amd64.tgz jujud
tar czf juju-2.9.1-windows-amd64.tgz jujud.exe
rm jujud jujud.exe
cd ~
juju-metadata generate-agents -d $HOME/simplestreams --stream proposed

Copy the tools folder to a web server that is accessible to your openstack environment, then bootstrap using:

juju bootstrap --debug --verbose --show-log \
    --config image-metadata-url="http://YOUR_SERVER_IP/images/" \
    --config agent-metadata-url="http://YOUR_SERVER_IP/tools/" \
    --config agent-stream=proposed oss oss-ctrl

Once bootstrapped, make sure the checksum of the jujud binary on the controller matches the one you built (got bit by this one a few times).

If it still fails, please post the log from:

C:\Program Files\Cloudbase Solutions\Cloudbase-Init\log\cloudbase-init.log

Also, on the windows instance, you can check the existance of the windows service representing the agent:

Get-Service juju*

To get info about the service:

sc.exe qc jujud-machine-0

@hmlanigan
Copy link
Member

QA worked today. Maybe it took the OpenStack I was using to be upgraded?

hmlanigan
hmlanigan approved these changes May 20, 2021
View reviewed changes
Copy link
Member

@hmlanigan hmlanigan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA good. Thank you for the contribution.

@hmlanigan
Copy link
Member

$$merge$$

@jujubot jujubot merged commit e1cb593 into juju:2.9 May 20, 2021
@wallyworld wallyworld mentioned this pull request Jun 4, 2021
Merged
jujubot added a commit that referenced this pull request Jun 6, 2021
@jujubot
Merge pull request #13050 from wallyworld/merge-2.9-20210604
0cf55ea 
#13050

Merge 2.9

#12859 Fix windows userdata
#12986 Prevent contention error when switching charms that drop peer relations
#13001 Charmhub: Improve download logging on failure
#13003 Only run upgrade-charm hook once when k8s charm upgraded
#13004 Update Pebble: enable "pebble run --verbose" logging and starting services as different user/group
#13007 Lp1920820 dashboard proxy support
#13002 CLI: Fix panic in upgrade-series with older controllers
#13036 Use mongo storage for external identity macaroon keys
#13040 Integration tests: Extend timeout
#13033 Remove stand-alone unit agent concerns
#13037 Handle SIGTERM and stop hook on sidecar deploys
#13038 Use the address CIDRs when populating machine addresses in the instance-poller
#13039 Fixes error with ensure ClusterRole
#13041 Update apt mirrors on all nodes when model value changes
#13043 Integration tests: Manual test charm selection
#13045 Fix race in caasunitterminationworker tests
#13044 Do not attempt downgrades in the upgrade-steps worker
#13046 Fix display of scale and workload version in status
#13048 Data race state/logdb tests
#13049 Report service address for sidecar k8s charms
#13031 vsphere values for show-cloud --include-config


```
# Conflicts:
# apiserver/facades/client/application/update_series_mocks_test.go
# apiserver/facades/client/bundle/bundle.go
# apiserver/facades/client/machinemanager/types_mock_test.go
# cmd/juju/application/bundle/bundle.go
# cmd/juju/application/bundle/bundle_test.go
# cmd/juju/application/deployer/bundlehandler.go
# cmd/juju/application/diffbundle.go
# cmd/juju/dashboard/dashboard.go
# featuretests/upgrade_test.go
# go.mod
# go.sum
# scripts/win-installer/setup.iss
# snap/snapcraft.yaml
# state/action_test.go
# version/version.go
# worker/uniter/resolver/loop.go
```

## QA steps

See PRs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hmlanigan hmlanigan hmlanigan approved these changes

Assignees
No one assigned
Labels
2.9
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gabriel-samfira @hmlanigan @hpidcock @jujubot