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

Adding support for TMC-Plugin discovery #2950

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

pgandigesang
Copy link
Contributor

@pgandigesang pgandigesang commented Jul 15, 2022

What this PR does / why we need it

This change is required to add support for TMC-Plugin discovery.

Which issue(s) this PR fixes

Fixes #3204

Describe testing done for PR

Release note

Added discovery source for TMC.
Fixed fetching artifacts over http to handle large file reads by processing chunks at a time.

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

@pgandigesang pgandigesang requested review from a team as code owners July 15, 2022 17:54
@pgandigesang pgandigesang force-pushed the tmc-discovery branch 3 times, most recently from a07b573 to ebf57fe Compare July 18, 2022 18:00
@rajathagasthya
Copy link
Member

Can you create a separate Github issue for this and link it in the PR description please?

apis/config/v1alpha1/clientconfig.go Outdated Show resolved Hide resolved
apis/config/v1alpha1/clientconfig.go Outdated Show resolved Hide resolved
pkg/v1/cli/command/core/root.go Outdated Show resolved Hide resolved
pkg/v1/cli/command/core/context.go Outdated Show resolved Hide resolved
pkg/v1/config/context.go Outdated Show resolved Hide resolved
@pgandigesang pgandigesang force-pushed the tmc-discovery branch 5 times, most recently from 9a9c3ee to 12654d4 Compare August 24, 2022 20:41
@pgandigesang pgandigesang force-pushed the tmc-discovery branch 3 times, most recently from a90d96f to 2302ca3 Compare August 29, 2022 20:42
@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

Merging #2950 (2302ca3) into main (254f018) will increase coverage by 0.12%.
The diff coverage is 42.50%.

❗ Current head 2302ca3 differs from pull request most recent head 92101e1. Consider uploading reports for the commit 92101e1 to get more accurate results

@@            Coverage Diff             @@
##             main    #2950      +/-   ##
==========================================
+ Coverage   45.82%   45.95%   +0.12%     
==========================================
  Files         361      391      +30     
  Lines       37642    40566    +2924     
==========================================
+ Hits        17249    18641    +1392     
- Misses      18820    20216    +1396     
- Partials     1573     1709     +136     
Impacted Files Coverage Δ
pkg/v1/cli/artifact/http.go 0.00% <0.00%> (ø)
pkg/v1/config/defaults.go 40.00% <0.00%> (-2.31%) ⬇️
pkg/v1/config/clientconfig.go 44.55% <56.25%> (ø)
pkg/v1/cli/command/core/discovery_source.go 41.86% <100.00%> (+1.95%) ⬆️
pkg/v1/cli/catalog.go 50.51% <0.00%> (-3.44%) ⬇️
addons/controllers/machine_controller.go 65.65% <0.00%> (-3.04%) ⬇️
pkg/v1/tkg/tkgpackageclient/package_update.go 83.57% <0.00%> (-1.43%) ⬇️
pkg/v1/tkg/managementcomponents/helper.go 91.89% <0.00%> (-0.22%) ⬇️
pkg/v1/cli/cmd.go 0.00% <0.00%> (ø)
... and 52 more

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

@giri-varma
Copy link
Contributor

LGTM @pgandigesang! Thanks for making the change

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.

This PR needs rebase. @pgandigesang Can you please help with the rebase as some of the code base has moved to cli/runtime go module.

apis/config/v1alpha1/clientconfig_types.go Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Sep 2, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2950/20220902202229/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

github-actions bot commented Sep 2, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/2950/20220902204949/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.

@pgandigesang pgandigesang requested review from anujc25 and giri-varma and removed request for anujc25 and giri-varma September 2, 2022 21:09
@anujc25
Copy link
Contributor

anujc25 commented Sep 2, 2022

I am confused why this commit 4cc3008 and changes related to this commit is shown as part of this PR? cc @pgandigesang

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.

There is some issue with the rebase as some additional changes are shown as diff with this PR. But otherwise changes looks good.

Signed-off-by: prajwal gandige sangamesh <pgandigesang@vmware.com>
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.

@anujc25 anujc25 added kind/feature Categorizes issue or PR as related to a new feature ok-to-merge PRs should be labelled with this before merging area/core-cli labels Sep 2, 2022
Copy link
Contributor

@giri-varma giri-varma 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!

@anujc25 anujc25 merged commit 7a25ddc into vmware-tanzu:main Sep 2, 2022
vuil pushed a commit that referenced this pull request Sep 21, 2022
Signed-off-by: prajwal gandige sangamesh <pgandigesang@vmware.com>

Signed-off-by: prajwal gandige sangamesh <pgandigesang@vmware.com>
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area/core-cli cla-not-required kind/feature Categorizes issue or PR as related to a new feature ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add TMC discovery source
6 participants