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

ConfigDrive support + fix on dns search #553

Merged
merged 5 commits into from
Jan 17, 2018
Merged

ConfigDrive support + fix on dns search #553

merged 5 commits into from
Jan 17, 2018

Conversation

jellonek
Copy link
Contributor

@jellonek jellonek commented Jan 14, 2018

TODO:

  • update integration test for ConfigDrive
  • update docs

This change is Reviewable

@jellonek jellonek force-pushed the jell/confdrive branch 7 times, most recently from 12e176d to 8787842 Compare January 14, 2018 23:17
@jellonek jellonek changed the title [WIP] ConfigDrive support + fix on dns search ConfigDrive support + fix on dns search Jan 15, 2018
@jellonek
Copy link
Contributor Author

Ok. It's ready to review.

@ivan4th
Copy link
Contributor

ivan4th commented Jan 15, 2018

Review status: 0 of 23 files reviewed at latest revision, 4 unresolved discussions.


docs/cloud-init-data-generation.md, line 20 at r1 (raw file):

ConfigDrive. Which one should be used is controlled by annotation
`VirtletCloudInitImageType`. It accepts as values `nocloud`, `configdrive` and
defaults to `nocloud` when annotation is not set.

Virtlet supports two types of Cloud-init ISO9660-based datasources, NoCloud and
ConfigDrive. The user may choose an appropriate one using
VirtletCloudInitImageType annotation, which can have either nocloud or configdrive as its value.
When there's no VirtletCloudInitImageType annotation, Virtlet defaults to nocloud.


docs/cloud-init-data-generation.md, line 160 at r1 (raw file):

In case of Config Drive this JSON has duplicated `instance-id` as `uuid` and
`local-hostname` as `hostname`.

In case of ConfigDrive, this JSON has instance-id repeated as uuid and local-hostname repeated as hostname.


pkg/libvirttools/cloudinit_test.go, line 99 at r1 (raw file):

		expectedNetworkConfig map[string]interface{}
		expectedUserDataStr   string
	}{

Is it possible to add a configdrive test case(s) here ? It shouldn't be hard


pkg/libvirttools/cloudinit_test.go, line 364 at r1 (raw file):

		},
		{
			name: "pod with network config",

These test cases need to be implemented for "configdrive", too


Comments from Reviewable

@jellonek
Copy link
Contributor Author

Review status: 0 of 23 files reviewed at latest revision, 4 unresolved discussions.


docs/cloud-init-data-generation.md, line 20 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Virtlet supports two types of Cloud-init ISO9660-based datasources, NoCloud and
ConfigDrive. The user may choose an appropriate one using
VirtletCloudInitImageType annotation, which can have either nocloud or configdrive as its value.
When there's no VirtletCloudInitImageType annotation, Virtlet defaults to nocloud.

Done.


docs/cloud-init-data-generation.md, line 160 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

In case of ConfigDrive, this JSON has instance-id repeated as uuid and local-hostname repeated as hostname.

Done.


pkg/libvirttools/cloudinit_test.go, line 99 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

Is it possible to add a configdrive test case(s) here ? It shouldn't be hard

Done.


pkg/libvirttools/cloudinit_test.go, line 364 at r1 (raw file):

Previously, ivan4th (Ivan Shvedunov) wrote…

These test cases need to be implemented for "configdrive", too

Done.


Comments from Reviewable

@ivan4th
Copy link
Contributor

ivan4th commented Jan 17, 2018

:lgtm:


Review status: 0 of 23 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@pigmej
Copy link
Contributor

pigmej commented Jan 17, 2018

:lgtm:


Reviewed 22 of 24 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jellonek
Copy link
Contributor Author

Rebased on top of master. Please wait for tests and rereview.

@ivan4th
Copy link
Contributor

ivan4th commented Jan 17, 2018

:lgtm:


Review status: 21 of 23 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@pigmej
Copy link
Contributor

pigmej commented Jan 17, 2018

:lgtm:


Reviewed 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@pigmej pigmej merged commit e2eb022 into master Jan 17, 2018
@pigmej pigmej deleted the jell/confdrive branch January 17, 2018 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants