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

Fix error on rke2 cluster for Azure #876

Merged
merged 3 commits into from
Jul 14, 2022
Merged

Fix error on rke2 cluster for Azure #876

merged 3 commits into from
Jul 14, 2022

Conversation

02bensch
Copy link
Contributor

@02bensch 02bensch commented Feb 25, 2022

This addresses and fixes #875
After adding the private_address_only to the schema, we were able to successfully create the resource.

Copy link

@phillipsj phillipsj left a comment

Choose a reason for hiding this comment

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

I just ran all the tests and they are all passing.

@phillipsj phillipsj requested a review from a team May 20, 2022 12:48
Copy link

@superseb superseb left a comment

Choose a reason for hiding this comment

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

add private_address_only
@02bensch
Copy link
Contributor Author

@superseb I have added the docs change.
There is however - I believe - some overlap between no_public_ip and private_address_only within the azure_config arguments. Perhaps in future no_public_ip could be removed, in order to align azure_config more closely with amazonec2_config arguments.

@eliyamlevy
Copy link
Contributor

eliyamlevy commented Jul 13, 2022

@02bensch I agree that we can probably drop the no_public_ip flag. Seems like overlapping functionality. I'd open another issue for removing it so it's written down somewhere.

@superseb superseb requested review from superseb and removed request for superseb July 13, 2022 16:43
@02bensch
Copy link
Contributor Author

@02bensch I agree that we can probably drop the no_public_ip flag. Seems like overlapping functionality. I'd open another issue for removing it so it's written down somewhere.

@eliyamlevy I have opened #956. I will link the necessary PRs to it when I get a chance.

@eliyamlevy
Copy link
Contributor

@02bensch sounds good. I've passed it along to the right people to get it approved before merging. We also have an internal issue for this but feel free to link yours as well.

@a-blender a-blender merged commit d13b63a into rancher:master Jul 14, 2022
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.

Error when creating rancher2_machine_config_v2 using Azure
5 participants