Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

New modules: tkr, object-propagation, util #3475

Merged
merged 17 commits into from
Oct 7, 2022

Conversation

imikushin
Copy link
Contributor

What this PR does / why we need it

Create new top-level modules: tkr, object-propagation and util.

Which issue(s) this PR fixes

N/A

Describe testing done for PR

Release note

Create new top-level modules: tkr, object-propagation and util.

Additional information

Special notes for your reviewer

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220922233903/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220922234643/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220923001043/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Ivan Mikushin added 2 commits September 23, 2022 13:52
Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
These packages are dead code: we're not using them.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@imikushin imikushin added the ok-to-merge PRs should be labelled with this before merging label Sep 23, 2022
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220923211011/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220923213330/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Merging #3475 (0e3cb04) into main (59f44fd) will decrease coverage by 2.36%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##             main    #3475      +/-   ##
==========================================
- Coverage   47.19%   44.82%   -2.37%     
==========================================
  Files         423      423              
  Lines       42307    40957    -1350     
==========================================
- Hits        19966    18361    -1605     
- Misses      20579    20917     +338     
+ Partials     1762     1679      -83     
Impacted Files Coverage Δ
apis/run/v1alpha3/kappcontrollerconfig_types.go 100.00% <ø> (ø)
tkg/clusterclient/clusterclient.go 49.28% <0.00%> (ø)
tkg/vsphere-template-resolver/template/resolver.go 89.91% <ø> (ø)
...re-template-resolver/templateresolver/interface.go 100.00% <ø> (ø)
...emplate-resolver/templateresolver/resolver_impl.go 99.02% <ø> (ø)
...sphere-template-resolver/templateresolver/types.go 42.10% <ø> (ø)
cmd/cli/plugin/cluster/available_upgrade.go 16.32% <100.00%> (ø)
cmd/cli/plugin/cluster/upgrade.go 57.55% <100.00%> (ø)
addons/controllers/clusterbootstrap_controller.go 63.26% <0.00%> (-1.61%) ⬇️
cmd/cli/plugin/tkr/v1alpha3/os.go 73.50% <0.00%> (-0.86%) ⬇️
... and 43 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@tenczar tenczar left a comment

Choose a reason for hiding this comment

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

LGTM. One question about the changes to the files in the tkg/web directory. Are those meant to be there? Can you still build the cli and run the UI with these changes?

Copy link
Contributor

@sathyanarays sathyanarays left a comment

Choose a reason for hiding this comment

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

@imikushin , should pkg/v1/tkr go into its own go module?

Copy link
Contributor

@sathyanarays sathyanarays left a comment

Choose a reason for hiding this comment

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

Will the tkr CLI be made as a separate go module?

Copy link
Contributor

@prkalle prkalle left a comment

Choose a reason for hiding this comment

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

The changes look good to me. I have couple of minor comments. Thanks!

featuregates/client/go.mod Outdated Show resolved Hide resolved
packages/package-values.yaml Show resolved Hide resolved
Ivan Mikushin added 4 commits September 26, 2022 18:20
Signed-off-by: Ivan Mikushin <imikushin@vmware.com>

# Conflicts:
#	go.mod
#	tkg/go.mod
Remove leftover tkr-vsphere-machine package.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220927015410/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220928004324/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220928012359/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

# Conflicts:
#	cli/core/go.mod
#	tkr/controller/tkr-source/constants/constants.go
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220929174533/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Make pkg/v1/tkr a module.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220929220710/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20220929223422/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Copy link
Contributor

@anujc25 anujc25 left a comment

Choose a reason for hiding this comment

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

Changes looks good.

Copy link
Contributor

@prkalle prkalle left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you @imikushin !

Copy link
Member

@yharish991 yharish991 left a comment

Choose a reason for hiding this comment

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

Looks good to me in general. Can you also update the CODEOWNERS file to reflect these changes?

@@ -0,0 +1,135 @@
// Copyright 2022 VMware, Inc. All Rights Reserved.
Copy link
Member

@yharish991 yharish991 Sep 30, 2022

Choose a reason for hiding this comment

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

this seems to be specific to tkr, should it be part of the tkr module? I follow this from Dave Cheney and always worked out well for me

@@ -6,16 +6,10 @@ package topology
import (
Copy link
Member

Choose a reason for hiding this comment

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

same comment as above

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>

# Conflicts:
#	go.mod
#	go.sum
@github-actions
Copy link

github-actions bot commented Oct 4, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20221004010747/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>

# Conflicts:
#	go.mod
#	go.sum
#	pkg/v2/tkr/webhook/infra-machine/Dockerfile
#	pkg/v2/tkr/webhook/tkr-conversion/Dockerfile
#	tkg/go.sum
@github-actions
Copy link

github-actions bot commented Oct 5, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20221005221033/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>

# Conflicts:
#	go.mod
#	go.sum
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20221006175547/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

This reverts commit b6bc162

Signed-off-by: Ivan Mikushin <imikushin@vmware.com>
@imikushin imikushin requested review from a team as code owners October 6, 2022 19:54
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20221006200448/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

# Conflicts:
#	pkg/v2/tkr/webhook/infra-machine/Dockerfile
#	pkg/v2/tkr/webhook/tkr-conversion/Dockerfile
@github-actions
Copy link

github-actions bot commented Oct 6, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3475/20221006214011/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@imikushin imikushin merged commit d2d9145 into vmware-tanzu:main Oct 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants