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

[JUJU-386] Improve yaml parsing for yaml files without 'clouds:' keyword ... #13597

Merged
merged 3 commits into from Jan 18, 2022

Conversation

cderici
Copy link
Member

@cderici cderici commented Jan 7, 2022

Description

This improves the ParseCloudMetadata to be able to parse yaml files for the cloudset that doesn't have the keyword clouds: at the top level.

The change is requested in the LP Bug #1826957

QA Steps

juju list-clouds -c <controller> --format yaml > /tmp/clouds.yaml
juju update-cloud -f /tmp/clouds.yaml localhost

Notes & Discussion

It is also discussed in the LP bug as another solution to put clouds: keyword into the list-cloud, but that'd break backwards compatibility.

@cderici cderici changed the title Improve yaml parsing for yaml files without 'clouds:' keyword ... [JUJU-386] Improve yaml parsing for yaml files without 'clouds:' keyword ... Jan 7, 2022
@hpidcock hpidcock added the 2.9 label Jan 9, 2022
Copy link
Member

@SimonRichardson SimonRichardson left a comment

Choose a reason for hiding this comment

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

We should attempt to coerce the format based on the schema. As empty lines are valid yaml, this is susceptible to failure. Instead, we should be working with the final representation.

This breaks your setup easily:

juju list-clouds -c <controller> --format yaml > /tmp/clouds.yaml
sed -i '0,/^/s//#some comment\n/' /tmp/clouds.yaml
juju update-cloud -f /tmp/clouds.yaml localhost

@cderici
Copy link
Member Author

cderici commented Jan 13, 2022

!!build!!

Copy link
Member

@SimonRichardson SimonRichardson left a comment

Choose a reason for hiding this comment

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

Last change, otherwise it looks great

cloud/clouds.go Outdated Show resolved Hide resolved
@cderici
Copy link
Member Author

cderici commented Jan 14, 2022

!!build!!

@cderici
Copy link
Member Author

cderici commented Jan 18, 2022

!!build!!

@cderici
Copy link
Member Author

cderici commented Jan 18, 2022

$$merge$$

@jujubot jujubot merged commit 925ad9b into juju:2.9 Jan 18, 2022
jujubot added a commit that referenced this pull request Jan 25, 2022
#13644

Merge from 2.9 to bring forward:
- #13643 from manadart/2.9-late-unit-public-addr
- #13641 from ycliuhw/fix/nw-deploy-bionic-aks
- #13638 from ycliuhw/fix/lp-1896209
- #13637 from wallyworld/external-controller-error
- #13619 from hmlanigan/bug-1956975-improved-logging
- #13632 from manadart/2.9-report-test-race-fix
- #13631 from manadart/2.9-remove-old-network-stuff
- #13597 from cderici/yaml-without-clouds-keyword
- #13626 from jack-w-shaw/JUJU-256_fix_smoke_race_condition
- #13611 from jack-w-shaw/JUJU-416_Consolidate_retries
- #13629 from SimonRichardson/report-model-cache
- #13598 from benhoyt/configurable-log-size
- #13625 from manadart/2.9-instance-poller-no-fake-addrs
- #13623 from jack-w-shaw/JUJU-416_Consolidate_retries_service
- #13622 from SimonRichardson/fix-state-tracker-panic
- #13620 from wallyworld/migration-with-cmr
- #13616 from wallyworld/peergroup-unittext-fixes
- #13612 from manadart/2.9-state-package-col-iter-close
- #13610 from jujubot/increment-to-2.9.24
- #13605 from SimonRichardson/fix-upgrade-closer-leak

Conflicts:
- apiserver/facades/agent/caasapplication/application_test.go
- cloudconfig/windows_userdata_test.go
- cmd/juju/commands/main.go
- cmd/juju/gui/gui.go
- cmd/juju/ssh/debugcode.go
- cmd/juju/ssh/debugcode_test.go
- cmd/juju/ssh/debughooks.go
- cmd/juju/ssh/debughooks_test.go
- cmd/juju/ssh/scp.go
- cmd/juju/ssh/scp_unix_test.go
- cmd/juju/ssh/ssh.go
- cmd/juju/ssh/ssh_machine.go
- cmd/juju/ssh/ssh_unix_test.go
- scripts/win-installer/setup.iss
- snap/snapcraft.yaml
- state/mocks/database_mock.go
- state/mocks/txn_mock.go
- state/prune.go
- version/version.go
- worker/containerbroker/mocks/agent_mock.go
- worker/containerbroker/mocks/machine_mock.go
- worker/instancemutater/mocks/agent_mock.go
@cderici cderici deleted the yaml-without-clouds-keyword branch February 16, 2022 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants