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

Renderer for kubernetes metadata #4675

Merged
merged 24 commits into from
Nov 28, 2022
Merged

Renderer for kubernetes metadata #4675

merged 24 commits into from
Nov 28, 2022

Conversation

lakshmimsft
Copy link
Contributor

@lakshmimsft lakshmimsft commented Nov 17, 2022 •
edited
Loading

Description

This PR creates renderers for Kubernetes Metadata extension. We will begin with first making it work for Containers and then update to cascade values from Env and Application extensions if they exist.

4351

Fixes: #4353

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Adds necessary unit tests for change
  • Adds necessary E2E tests for change
  • Unit tests passing
  • Extended the documentation / Created issue for it

@lakshmimsft lakshmimsft force-pushed the ljavadekar/kubemetadataextrenderer branch from c3a701a to b51445b Compare November 17, 2022 17:25
ytimocin
ytimocin reviewed Nov 18, 2022
View reviewed changes
pkg/corerp/backend/deployment/deploymentprocessor.go Outdated Show resolved Hide resolved
pkg/corerp/backend/deployment/deploymentprocessor.go Outdated Show resolved Hide resolved
pkg/corerp/backend/deployment/deploymentprocessor.go Outdated Show resolved Hide resolved
pkg/corerp/backend/deployment/deploymentprocessor.go Outdated Show resolved Hide resolved
pkg/corerp/backend/deployment/deploymentprocessor.go Outdated Show resolved Hide resolved
pkg/corerp/renderers/kubernetesmetadata/render.go Outdated Show resolved Hide resolved
pkg/corerp/renderers/kubernetesmetadata/render.go Outdated Show resolved Hide resolved
pkg/corerp/renderers/kubernetesmetadata/render.go Outdated Show resolved Hide resolved
pkg/corerp/renderers/kubernetesmetadata/render_test.go Outdated Show resolved Hide resolved
pkg/corerp/renderers/types.go Outdated Show resolved Hide resolved
@lakshmimsft lakshmimsft force-pushed the ljavadekar/kubemetadataextrenderer branch from bd39b6c to 66a6c01 Compare November 21, 2022 04:40
@lakshmimsft lakshmimsft self-assigned this Nov 21, 2022
@lakshmimsft
Copy link
Contributor Author

Updating to 'ready for review'.
Code & Tests were updated to read and set both Metadata and Spec level Labels and Annotations.

@lakshmimsft lakshmimsft marked this pull request as ready for review November 21, 2022 06:42
@lakshmimsft lakshmimsft requested a review from a team as a code owner November 21, 2022 06:42
jkotalik
jkotalik reviewed Nov 21, 2022
View reviewed changes
Copy link
Contributor

@jkotalik jkotalik left a comment

Choose a reason for hiding this comment

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

Great progress on this, some feedback.

vinayada1
vinayada1 reviewed Nov 21, 2022
View reviewed changes
pkg/corerp/renderers/kubernetesmetadata/render.go Outdated

// mergeMaps merges meta, spec annotations/labels in the sequence env+app map->current map
func mergeKubernetesMetadataMaps(cascadeMap map[string]string, currMap map[string]string, metaMap map[string]string, specMap map[string]string) (map[string]string, map[string]string) {
currMap = labels.Merge(cascadeMap, currMap)
Copy link
Contributor

@vinayada1 vinayada1 Nov 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

What is the above line for?

@lakshmimsft lakshmimsft force-pushed the ljavadekar/kubemetadataextrenderer branch from d00d41c to d51895b Compare November 21, 2022 23:29
jkotalik
jkotalik reviewed Nov 22, 2022
View reviewed changes
pkg/corerp/renderers/kubernetesmetadata/render.go
setAnnotations(o, metaAnnotations, specAnnotations)
}

if kubeMetadataExt != nil && kubeMetadataExt.Labels != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same applies to these as well.

Copy link
Contributor Author

@lakshmimsft lakshmimsft Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is required because in this update after line 56, we don't return if the extension doesn't exist we continue ahead so kubeMetadataExt could be nil

@lakshmimsft lakshmimsft force-pushed the ljavadekar/kubemetadataextrenderer branch from 8beea8d to 7ce13d5 Compare November 28, 2022 17:45
jkotalik
jkotalik approved these changes Nov 28, 2022
View reviewed changes
Copy link
Contributor

@jkotalik jkotalik left a comment

Choose a reason for hiding this comment

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

Great stuff!

Lakshmi Javadekar added 24 commits November 28, 2022 14:03
initial commit
3f4a0e5
initial unit test
9837e2b
cast to unstructured.Unstructured doesn't work
9b1d8e0
upded app model
818c653
tests running with cast to v1/Deployment
e1761fc
tests running with cast v1/Deployment
e862873
populating app/env options with extensions data
6e58be7
updated per comments
fd412b3
correct input variable
a82d88c
intermediate update, updating meta lbl, ann in addition to spec
79064ac
intermediate update, cascading merge env->app->container
f663680
intermediate update, more tests needed
fc44b34
updated tests
c682cd3
test updates
ef4e238
nit update
729f32c
add collision test
5e52df2
nit
9e570d9
nit test
331ddfe
updated per comments
ef8e9ce
nit comment
e21b990
updates per recent updates to datamodel
8f50cae
updating per comments
824ed61
nit comment
3f80c65
rebase with main
af16abf
@lakshmimsft lakshmimsft force-pushed the ljavadekar/kubemetadataextrenderer branch from 7ce13d5 to af16abf Compare November 28, 2022 22:24
@lakshmimsft lakshmimsft merged commit d6efff6 into main Nov 28, 2022
@lakshmimsft lakshmimsft deleted the ljavadekar/kubemetadataextrenderer branch November 28, 2022 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vinayada1 vinayada1 vinayada1 left review comments

@youngbupark youngbupark youngbupark left review comments

@ytimocin ytimocin ytimocin left review comments

@jkotalik jkotalik jkotalik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@lakshmimsft @ytimocin @jkotalik @youngbupark @vinayada1