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

use build tooling for building capabilities component #4307

Conversation

yharish991
Copy link
Member

@yharish991 yharish991 commented Jan 30, 2023

What this PR does / why we need it

This pr uses build tooling for building capabilities component

Which issue(s) this PR fixes

Fixes #4414

Describe testing done for PR

Ran make docker-build-all -f build-tooling.mk from the project's root directory and verified that the capabilities component is built.

Release note


Additional information

Special notes for your reviewer

@yharish991 yharish991 requested review from a team as code owners January 30, 2023 16:30
@yharish991 yharish991 marked this pull request as draft January 30, 2023 16:30
@yharish991 yharish991 force-pushed the useBuildToolingForCapabilities branch 8 times, most recently from 8cc0471 to 0f72d1d Compare February 14, 2023 23:57
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4307/20230215034227/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 Feb 15, 2023

Codecov Report

Merging #4307 (9ee961e) into feature/runtime-core (e815dca) will decrease coverage by 0.88%.
The diff coverage is n/a.

❗ Current head 9ee961e differs from pull request most recent head 5abfcaa. Consider uploading reports for the commit 5abfcaa to get more accurate results

@@                   Coverage Diff                    @@
##           feature/runtime-core    #4307      +/-   ##
========================================================
- Coverage                 49.60%   48.72%   -0.88%     
========================================================
  Files                       452      482      +30     
  Lines                     45044    47175    +2131     
========================================================
+ Hits                      22344    22987     +643     
- Misses                    20577    22015    +1438     
- Partials                   2123     2173      +50     
Impacted Files Coverage Δ
capabilities/client/pkg/discovery/fake.go 100.00% <ø> (ø)
...ons/controllers/packageinstallstatus_controller.go 77.99% <0.00%> (-2.32%) ⬇️
...cluster/delete_machinehealthcheck_control_plane.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/get_node_pools.go 10.52% <0.00%> (ø)
cmd/cli/plugin/cluster/scale.go 17.85% <0.00%> (ø)
.../cli/plugin/cluster/get_machinehealthcheck_node.go 9.30% <0.00%> (ø)
cmd/cli/plugin/cluster/list.go 11.36% <0.00%> (ø)
cmd/cli/plugin/cluster/get_machinehealthcheck.go 11.42% <0.00%> (ø)
cmd/cli/plugin/cluster/delete_node_pool.go 16.66% <0.00%> (ø)
cmd/cli/plugin/cluster/credentials_update.go 8.73% <0.00%> (ø)
... and 26 more

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

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/4307/20230217031057/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/4307/20230217194227/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/4307/20230217201140/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.

@yharish991 yharish991 marked this pull request as ready for review February 20, 2023 05:31
@yharish991 yharish991 requested review from a team as code owners February 20, 2023 05:31
@yharish991
Copy link
Member Author

yharish991 commented Feb 20, 2023

CAPD integration tests have been consistently failing for some reason, it looks unrelated to the changes in this pr

@yharish991 yharish991 changed the title [WIP] use build tooling for building capabilities image use build tooling for building capabilities image Feb 21, 2023
@@ -0,0 +1,75 @@
# Copyright 2023 VMware, Inc. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Purely from user experience perspective, is it possible to rename this Dockerfile to be something else from which people understand this Dockerfile cannot be directly used by docker build under T-F root. (Or we could add some inline comments?)

The reason why I am raising this is because I got confused at the beginning until I looked into the dockerfile content.

Copy link
Member Author

Choose a reason for hiding this comment

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

docker-build make target in t-f Makefile calls the docker-build target command in each component's Makefile, but I can add a comment to provide more clarity

Copy link
Member Author

Choose a reason for hiding this comment

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

added the comment

@yharish991 yharish991 changed the title use build tooling for building capabilities image use build tooling for building capabilities component Feb 22, 2023
@@ -0,0 +1,297 @@
.DEFAULT_GOAL := help
Copy link
Contributor

Choose a reason for hiding this comment

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

question: when we make changes in https://github.com/vmware-tanzu/build-tooling-for-integrations/tree/main/templates, or when user makes changes against this file in T-F by adding more components, what we can do to make things consistent ?

Copy link
Member Author

@yharish991 yharish991 Feb 22, 2023

Choose a reason for hiding this comment

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

it's up to the consumer(t-f) on when they want to consume the new release from https://github.com/vmware-tanzu/build-tooling-for-integrations, but when they update the Makefile in t-f it is their responsibility to update the components var, but this will change later when the design gets approved and implemented(we will have a config file to maintain components)

@github-actions
Copy link

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

IMG_VERSION_OVERRIDE ?= $(IMG_DEFAULT_TAG)
GOPROXY ?= "https://proxy.golang.org,direct"
PLATFORM=local
COMPONENTS ?= capabilities/client capabilities/controller.capabilities-controller-manager.capabilities
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment about what this line means and how COMPONENTS is supposed to be used?

Copy link
Member Author

Choose a reason for hiding this comment

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

added a comment that provides link to the documentation

@github-actions
Copy link

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

@yharish991 yharish991 changed the base branch from main to feature/runtime-core February 24, 2023 17:45
@rajathagasthya rajathagasthya added the ok-to-merge PRs should be labelled with this before merging label Feb 24, 2023
@yharish991 yharish991 merged commit 5de127a into vmware-tanzu:feature/runtime-core Feb 26, 2023
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.

Use build tooling to build capabilities component
5 participants