-
Notifications
You must be signed in to change notification settings - Fork 909
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
openstack: read the dynamic metadata group vendor_data2.json #777
Conversation
This has been present in the openestack metadata since version 'Newton'. Bug: 1841104
…ical#586)" (canonical#775) This reverts commit b0e7381.
Ubuntu cloud images ship /etc/cloud/build.info which includes a line with the build serial used to identify the image: serial: 20210108 This is valuable information when verifying Ubuntu issues (to confirm that testing is happening against the expected image), but is also useful when debugging test failures: manifests of all packages in (the base) images can be found at http://cloud-images.ubuntu.com/
The company name has two distinct words. Signed-off-by: Dan Kenigsberg <danken@redhat.com>
Two shell code blocks are not marked as such, confusing rst to consider them as yaml. Be explicit about their syntax, and use $ prompt to match elsewhere in the docs. /home/travis/build/canonical/cloud-init/doc/rtd/topics/format.rst:28: WARNING: Could not lex literal_block as "yaml". Highlighting skipped. /home/travis/build/canonical/cloud-init/doc/rtd/topics/format.rst:52: WARNING: Could not lex literal_block as "yaml". Highlighting skipped. Signed-off-by: Dan Kenigsberg <danken@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this. Dynamic vendor data will be very useful.
Overall, this change looks good. I have a few comments inline. Additionally,
- Can you update the documentation to add a comment about dynamic vendordata / vendordata2 to the Vendor Data section of openstack.rst
- Can you run this against an openstack system with dynamic vendordata and provide some logs (
cloud-init collect-logs
)? Has it been tested against a live system with dynamic vendor data overriding static vendor data, and then also the user data overriding the vendor data? Based on the code, I have no real reason to question these things, but it's always good to have verification against a real system.
I think I found a regression in the jinja handler (unrelated to my code but that interferes with getting a clean test). Please don't merge until I know what's up with that :) ETA: logged as https://bugs.launchpad.net/cloud-init/+bug/1914641 |
Here's the output from a test run of this:
|
I've logged the jinja regression as 1914641; I believe it's not a blocker for this patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the refactor! Just a couple minor changes left.
Co-authored-by: James Falcon <TheRealFalcon@users.noreply.github.com>
Thank you @TheRealFalcon |
@@ -392,6 +395,11 @@ def get_vendordata(self): | |||
self.vendordata = self.ud_proc.process(self.get_vendordata_raw()) | |||
return self.vendordata | |||
|
|||
def get_vendordata2(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should not have been put here. A single datasource (openstack) has a thing called 'vendordata2'. Shouldn't the change have been made in openstack datasource rather than propagating up to the base class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @smoser. I'm looking at how to move this handling into openstack-specific code and it's starting to seem like a pretty big refactor. It's easy enough to move the things from init.py into DataSourceOpenStack.py but then the vendordata2 handlers in stages.py will need to be made aware of which subclasses do or don't support vendordata2; failing that we could subclass things in stages.py but as best I can tell that isn't currently done anywhere so would require some additional scaffolding.
If you still want me to go ahead with refactoring I can work on it but I welcome your thoughts about how to avoid littering the existing code with checks :) It might also be worth creating a new bug to track.
(Of course when I wrote the initial patchset I was imagining that this might be a feature that other datasources would want to implement but that may not be realistic.)
-Andrew
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Of course when I wrote the initial patchset I was imagining that this might be a feature that other datasources would want to implement but that may not be realistic.)
I'm confused as to how that would be any different than vendor data
to cloud-init.
As far as rework, i would have thought that you would just change the datasource to say : If there is vendor-data2, present it as vendor-data, otherwise use vendor-data.
Hi @smoser @TheRealFalcon @andrewbogott Relevant part from the cloud-init.log looks like this:
It reads the 74 bytes but doesn't write the value (the YAML file) mapped to key "cloud-init" to the file to vendor-data2.txt, because it was not able to find "vendordata2 from datasource". My vendor_data2.json looks like this: Please let me know, if this issue is due to my JSON or an issue with how cloud-init handles vendor data. Since this pull request is fairly new, I am assuming not many people had the chance to test dynamic vendor data using cloud-init yet. |
@mmahalwa, I believe in your nova.conf you are setting your vendordata target like this: testing@http:// My understanding of this feature is that if you want the vendordata to be consumed by cloud-init you should instead direct it to the cloud-init target: cloud-init@http:// Then you can remove the explicit cloud-init dictionary key in your datasource so that you get something that looks like
I don't have a test platform handy to confirm this but I would expect that (or something very much like that) to work. |
@andrewbogott I can confirm that even after changing the json to Just for reference, this is how I am writing my response from my REST service to the POST request sent by Nova:
I had mentioned here bugs.launchpad, why name@ might have a different meaning. From (https://specs.openstack.org/openstack/nova-specs/specs/newton/implemented/vendordata-reboot.html) vendordata_dynamic_targets=name1@http://example.com,name2@http://example2.com The name element for these microservices becomes the new key for the vendor_data2.json file like,
Please note I have taken the above example from the Nova specs mentioned earlier and extended it for 2 REST services. Now, if we consider your approach of having cloud-init as the outer key, then how are we going to differentiate b/w several REST services. |
In #777, we added 'vendordata2' and 'vendordata2_raw' attributes to the DataSource class, but didn't use the upgrade framework to deal with an unpickle after upgrade. This commit adds the necessary upgrade code. Additionally, added a smaller-scope upgrade test to our integration tests that will be run on every CI run so we catch these issues immediately in the future. LP: #1922739
Almost year ago passed and seems still not resolved at all with vanilla openstack... nova-api is configured like next
Vendor data returns next response:
So everything is following OpenStack docs but no success due to cloud-init looks for "cloud-init" key in response, but no way to return string "#cloud-config" because from each service expected json reply. If you will try to return plain text from vendordata server (named as cloud-init to return as key
Did anybody found any workarounds on this ? Since feature is highly usefull it is not possible to use out-of-the-box due to misunderstand between cloud-init and openstack |
@frct1 Is there a github issue filed for this? If not, then please file one. If so, please continue the conversation there. You can always link to old PRs if they are relevant. If there are bugs to be fixed or features to add, we're happy to collaborate to find solutions. |
|
Add support for openstack's dynamic vendor data, which appears under openstack/latest/vendor_data2.json
This adds vendor_data2 to all pathways; it should be a no-op for non-OpenStack providers.
Bug: https://bugs.launchpad.net/cloud-init/+bug/1841104