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

241: Fix test vm creation with init #251

Merged
merged 2 commits into from
Aug 8, 2023
Merged

241: Fix test vm creation with init #251

merged 2 commits into from
Aug 8, 2023

Conversation

dyens
Copy link

@dyens dyens commented Aug 6, 2023

Please describe the change you are making

Live test TestVMCreationWithInit not passed. It should be fixed by:

  • Add nic.Name in vmbuilderInititalization
  • In convertSDK use sdkObj.Ip() and sdkObj.Ipv6() for version detecting

Ignore Ip().Version field because it have a nil value in ovirt response

Are you the owner of the code you are sending in, or do you have permission of the owner?

yes

The code will be published under the BSD 3 clause license. Have you read and understood this license?

yes

@dyens dyens mentioned this pull request Aug 6, 2023
Closed
@dyens dyens force-pushed the 241-fix-initialization-with-nic-live-tests branch from 915f4be to 3981a70 Compare August 6, 2023 07:19
@dyens dyens changed the title 241: Fix test vm createion with init 241: Fix test vm creation with init Aug 6, 2023
@dyens dyens marked this pull request as draft August 6, 2023 14:11
@dyens
Copy link
Author

dyens commented Aug 6, 2023

Looks like this is incorrect. I think that IPV4 is required for NicConfiguration and Ipv6 is optional.

Kapustin Aleksandr added 2 commits August 6, 2023 23:43
241: Fix test vm createion with init
9657086 
- Add nic.Name in vmbuilderInititalization
- In convertSDK use sdkObj.Ip() and sdkObj.Ipv6() for version detecting
(Ignore Ip().Version field. Is nil in ovirt response)

Signed-off-by: Kapustin Aleksandr <[email protected]>
241: Add ipv6 optional field to NicConfiguration
3c942a6 
Signed-off-by: Kapustin Aleksandr <[email protected]>
@dyens dyens force-pushed the 241-fix-initialization-with-nic-live-tests branch from 0d9bba5 to 3c942a6 Compare August 6, 2023 20:43
@dyens dyens marked this pull request as ready for review August 6, 2023 20:44
@dyens
Copy link
Author

dyens commented Aug 6, 2023

I added new optional IPV6 field to the NicConfiguration.
Every NicConfiguration have IP (ipv4) field (this is required by Ovirt - we have bad request if we try to send empty ipv4).

Added some test cases.
I think, this PR is ready for review. Please correct me.

@engelmi
Copy link
Contributor

engelmi commented Aug 7, 2023

@dyens Thank you for your contribution! This PR looks good to me. Have you run the live tests as well?

@dyens
Copy link
Author

dyens commented Aug 8, 2023

@dyens Thank you for your contribution! This PR looks good to me. Have you run the live tests as well?

Yes, TestVMCreationWithInit passed for me in live and mock modes.

(But i have some issues in live mode for some other tests - maybe this is a problem of my OVirt stand. This issues is present before this PR. I would like to debug it some day)

engelmi
engelmi approved these changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@engelmi engelmi left a comment

Choose a reason for hiding this comment

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

Great! In that case, lets get it merged :)

@engelmi engelmi merged commit a5d2d8c into oVirt:main Aug 8, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@engelmi engelmi engelmi approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@dyens @engelmi