-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
add open service mesh option for kuberentes service (public preview) #11189
add open service mesh option for kuberentes service (public preview) #11189
Conversation
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.
hey @yershalom
Thanks for this PR - taking a look through on the whole this looks pretty good to me, but can we add some documentation for these new fields so that users are able to find them?
Thanks!
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.
hey @yershalom
Thanks for pushing those changes, I've taken a look through and left a few more comments inline, but if we can fix those up (and the tests pass) then this should be good to go 👍
Thanks!
"enabled": { | ||
Type: schema.TypeBool, | ||
Required: true, | ||
}, |
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.
can we ensure these are set back into the state too? this is done in the flatten function below: https://github.com/terraform-providers/terraform-provider-azurerm/pull/11189/files#diff-c28ad69551916efc44930e639bf392636923aa37f4a68e17cb874edf2c34b101R312-R341
}, | ||
}, | ||
}, | ||
}, |
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.
we'll also need to flatten this value for the Data Source, this is done in flattenKubernetesClusterDataSourceAddonProfiles
(by parsing the add-on profile from the response and reading the enabled state)
per tombuildsstuff review Co-authored-by: Tom Harvey <[email protected]>
Hey @yershalom - just wondering if your still working on this 🙂 |
Hi @katbyte , yes, I have a lot of pressure now but I will fix the comment and conflicts when I'll have time :-) |
hey @yershalom - i'm going to close this for now as its been a month and the branch is getting out of date/conflicted - please do reopen this once you've had time to address the reviews - thanks again! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
https://techcommunity.microsoft.com/t5/apps-on-azure/announcing-public-preview-of-the-open-service-mesh-osm-aks-add/ba-p/2247361
https://docs.microsoft.com/en-us/azure/aks/servicemesh-osm-about?pivots=client-operating-system-linux