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

Add serviceAccount to ReadinessProvider spec #4637

Merged
merged 3 commits into from
Jun 28, 2023

Conversation

avi-08
Copy link
Contributor

@avi-08 avi-08 commented Jun 19, 2023

What this PR does / why we need it

ReadinessProvider spec accepts serviceAccount as user input for making k8s API calls to evaluate conditions.
Its an optional config, if not provided, the default controller service account is used.
Also adding an optional Message field in ReadinessProvider Status to contain overall status details.

Which issue(s) this PR fixes

Fixes #

Describe testing done for PR

Release note


Additional information

Special notes for your reviewer

@avi-08 avi-08 added area/api kind/api-change PR/Issue related to adding, removing, or changing an API labels Jun 19, 2023
@avi-08 avi-08 force-pushed the topic/avi-08/readiness-svc-acc branch from 562dfd5 to f6a8aa3 Compare June 19, 2023 06:47
@codecov
Copy link

codecov bot commented Jun 19, 2023

Codecov Report

Merging #4637 (40052a8) into main (7fa2b72) will increase coverage by 0.26%.
The diff coverage is 88.88%.

❗ Current head 40052a8 differs from pull request most recent head 514eaca. Consider uploading reports for the commit 514eaca to get more accurate results

@@            Coverage Diff             @@
##             main    #4637      +/-   ##
==========================================
+ Coverage   75.06%   75.33%   +0.26%     
==========================================
  Files          21       21              
  Lines        1199     1212      +13     
==========================================
+ Hits          900      913      +13     
  Misses        249      249              
  Partials       50       50              
Impacted Files Coverage Δ
.../readinessprovider/readinessprovider_controller.go 95.45% <87.50%> (-4.55%) ⬇️
...ess/controller/pkg/conditions/resourceexistence.go 90.32% <100.00%> (+6.98%) ⬆️

@avi-08 avi-08 marked this pull request as ready for review June 19, 2023 06:59
@avi-08 avi-08 requested a review from a team as a code owner June 19, 2023 06:59
@avi-08 avi-08 force-pushed the topic/avi-08/readiness-svc-acc branch 3 times, most recently from d5915f2 to b2c4e13 Compare June 19, 2023 08:05
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.

Overall, the changes look good. Only two concerns at high level.

readiness/controller/pkg/conditions/resourceexistence.go Outdated Show resolved Hide resolved
apis/core/v1alpha2/readinessprovider_types.go Outdated Show resolved Hide resolved
Signed-off-by: Avi Sharma <avi.08.sh@gmail.com>
@avi-08 avi-08 force-pushed the topic/avi-08/readiness-svc-acc branch 5 times, most recently from 518b5d4 to 81e3179 Compare June 20, 2023 12:31
Signed-off-by: Avi Sharma <avi.08.sh@gmail.com>
@avi-08 avi-08 force-pushed the topic/avi-08/readiness-svc-acc branch from 81e3179 to 365affc Compare June 20, 2023 12:37
apis/core/v1alpha2/readinessprovider_types.go Outdated Show resolved Hide resolved
apis/core/v1alpha2/readinessprovider_types.go Outdated Show resolved Hide resolved
apis/core/v1alpha2/readinessprovider_webhook.go Outdated Show resolved Hide resolved
readiness/controller/main.go Outdated Show resolved Hide resolved
util/config/config.go Outdated Show resolved Hide resolved
apis/core/v1alpha2/readinessprovider_webhook.go Outdated Show resolved Hide resolved
Signed-off-by: Avi Sharma <avi.08.sh@gmail.com>
@avi-08 avi-08 force-pushed the topic/avi-08/readiness-svc-acc branch from 40052a8 to 514eaca Compare June 21, 2023 08:55
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.

LGTM!!

@rajathagasthya rajathagasthya added the ok-to-merge PRs should be labelled with this before merging label Jun 27, 2023
@avi-08 avi-08 merged commit c9b1d31 into vmware-tanzu:main Jun 28, 2023
@avi-08 avi-08 deleted the topic/avi-08/readiness-svc-acc branch June 28, 2023 04:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/api cla-not-required kind/api-change PR/Issue related to adding, removing, or changing an API ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants