Added log message for API call failures #7834

prady0t · 2023-07-15T08:43:11Z

Explanation

There were no log messages for API call failures. dl.logger.Error(err, "failed to load data: ") now prints the error along with log message.

Related issue

Closes #7734
Closes #4774

Milestone of this PR

/milestone 1.10.2

What type of PR is this

Proposed Changes

Proof Manifests

Checklist

I have read the contributing guidelines.
I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
This is a bug fix and I have added unit tests that prove my fix is effective.
This is a feature and I have added CLI tests that are applicable.
My PR needs to be cherry picked to a specific release branch which is .
My PR contains new or altered behavior to Kyverno and
- CLI support should be added and my PR doesn't contain that functionality.
- I have added or changed the documentation myself in an existing PR and the link is:
- I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

This is the same PR #7792. Had to close that. Changes are exactly the same as before

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

sonatype-lift · 2023-07-15T08:43:14Z

Sonatype Lift is retiring

Sonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

📖 Read about the impacts and timeline

codecov · 2023-07-16T15:12:19Z

Codecov Report

Merging #7834 (993cb1f) into main (1fff5ba) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #7834      +/-   ##
==========================================
- Coverage   32.93%   32.93%   -0.01%     
==========================================
  Files         239      239              
  Lines       22510    22514       +4     
==========================================
+ Hits         7413     7414       +1     
- Misses      14336    14338       +2     
- Partials      761      762       +1

Impacted Files	Coverage Δ
pkg/engine/context/deferred.go	`77.88% <50.00%> (-2.12%)`	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

prady0t · 2023-07-16T16:14:50Z

@chipzoller why do some CI tests keep on failing?

realshuting

/lgtm

@JimBugwadia - please take a look.

JimBugwadia

Added a comment on the log syntax - please check.

pkg/engine/context/deferred.go

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

realshuting · 2023-07-19T08:45:54Z

/cherry-pick release-1.10

welcome · 2023-07-19T09:15:11Z

Congratulations! 🎉

Great job merging your first Pull Request here! How awesome! If you are new to this project, feel free to join our Slack community

gcp-cherry-pick-bot · 2023-07-19T09:15:15Z

Cherry-pick failed with Merge error d18a27d189a63f9310677dea6bb8c5eb1cb4a05e into temp-cherry-pick-98d409-release-1.10

realshuting · 2023-07-19T09:28:35Z

Good work @prady0t!

Could you cherry-pick this PR to release-1.10 branch? You can find details here.

prady0t · 2023-07-19T10:22:48Z

I cannot do that, checkout to release-1.10 banch from my local terminal throws this error.

❯ git checkout release-1.10
error: pathspec 'release-1.10' did not match any file(s) known to git

What am I doing wrong?

realshuting · 2023-07-19T12:49:19Z

@prady0t - you need to create the local release-1.10 branch from remote, see instructions here.

Could you cherry-pick this PR to release-1.10 branch? You can find details here.

Is there anything missing in this section? Would be good to fill the gap and add instructions if needed.

prady0t · 2023-07-19T14:36:48Z

@prady0t - you need to create the local release-1.10 branch from remote, see instructions here.

Could you cherry-pick this PR to release-1.10 branch? You can find details here.

Is there anything missing in this section? Would be good to fill the gap and add instructions if needed.

Yes I think few more instructions can be added. Will open a PR once done here.

prady0t · 2023-07-19T19:46:49Z

Hey @realshuting I tried to follow above link yet I'm unable to do it.

I did git fetch , git fetch --all now git branch -v -a shows:

Which is just a list of branches I made locally from main . I cannot see release-1.10 branch or any other branch from kyverno repository.
I cannot see these branches:

Am I supposed to create a branch named release-1.10 and cherry-pick my commits there and then push it? Or is it something else?
(These might be annoying questions but since I'm quite new to this, your help will go a long way.😅)

realshuting · 2023-07-20T10:19:36Z

Seems like you don't have remote upstream repo added, can you follow this post?

prady0t · 2023-07-20T15:20:14Z

That worked! Thanks @realshuting. Ok so the problem with cherry-pick is this:

In main branch:

kyverno/pkg/engine/factories/contextloaderfactory.go

Line 102 in f905f90

if rclientFactory != nil {

In release-1.10 branch:

kyverno/pkg/engine/factories/contextloaderfactory.go

Line 104 in 942061e

if rclient != nil {

Someone renamed rclientFactory to rclient and it's causing conflicts while cherry-pick. Should I rename rclientFactory in my original PR as well? How to solve this conflict?

prady0t · 2023-07-20T21:51:23Z

Seems like you don't have remote upstream repo added, can you follow this post?

Made a PR related to this: https://github.com/kyverno/kyverno/pull/7872/files

realshuting · 2023-07-21T02:58:35Z

That worked! Thanks @realshuting. Ok so the problem with cherry-pick is this:

In main branch:

kyverno/pkg/engine/factories/contextloaderfactory.go

Line 102 in f905f90

if rclientFactory != nil {

In release-1.10 branch:

kyverno/pkg/engine/factories/contextloaderfactory.go

Line 104 in 942061e

if rclient != nil {

Someone renamed rclientFactory to rclient and it's causing conflicts while cherry-pick. Should I rename rclientFactory in my original PR as well? How to solve this conflict?

You need to keep it as the same name on release-1.10 branch rclient so it won't impact any future PRs.

* Added error message to deferred loader on API call failure Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> * Small change in error message Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> --------- Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> Co-authored-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> Co-authored-by: Chip Zoller <chipzoller@gmail.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: Jim Bugwadia <jim@nirmata.com> Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

prady0t · 2023-07-21T08:02:06Z

@realshuting Made a PR on release-1.10 branch. #7880

* Added error message to deferred loader on API call failure * Small change in error message --------- Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

* Added error message to deferred loader on API call failure Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> * Small change in error message Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> --------- Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> Co-authored-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> Co-authored-by: Chip Zoller <chipzoller@gmail.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: Jim Bugwadia <jim@nirmata.com>

pkg/engine/context/deferred.go

Added error message to deferred loader on API call failure

02d4ba4

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

prady0t requested review from MarcelMue, realshuting and MariamFahmy98 as code owners July 15, 2023 08:43

prady0t mentioned this pull request Jul 15, 2023

Added log message for API call failures #7792

Closed

9 tasks

Merge branch 'main' into API-failure-fix

9a4c8b3

realshuting previously approved these changes Jul 17, 2023

View reviewed changes

realshuting added milestone 1.10.2 cherry-pick-required labels Jul 17, 2023

JimBugwadia requested changes Jul 17, 2023

View reviewed changes

pkg/engine/context/deferred.go Outdated Show resolved Hide resolved

Small change in error message

066a3f9

Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com>

prady0t dismissed realshuting’s stale review via 066a3f9 July 17, 2023 08:22

prady0t requested review from realshuting and JimBugwadia July 17, 2023 12:58

realshuting and others added 2 commits July 18, 2023 17:28

Merge branch 'main' into API-failure-fix

f67b7b8

Merge branch 'main' into API-failure-fix

2c01129

JimBugwadia approved these changes Jul 19, 2023

View reviewed changes

Merge branch 'main' into API-failure-fix

993cb1f

realshuting enabled auto-merge (squash) July 19, 2023 08:45

realshuting approved these changes Jul 19, 2023

View reviewed changes

realshuting merged commit d18a27d into kyverno:main Jul 19, 2023

realshuting mentioned this pull request Jul 21, 2023

Added log message for API call failures (#7834), cherry picked #7880

Merged

9 tasks

realshuting added the cherry-pick-completed The PR was cherry-picked (or merged) to required release branches label Jul 21, 2023

prady0t deleted the API-failure-fix branch July 21, 2023 09:06

realshuting reviewed Aug 8, 2023

View reviewed changes

pkg/engine/context/deferred.go Show resolved Hide resolved

realshuting mentioned this pull request Aug 8, 2023

fix: return err in load data #7982

Merged

Added log message for API call failures #7834

Added log message for API call failures #7834

Uh oh!

Conversation

prady0t commented Jul 15, 2023

Explanation

Related issue

Milestone of this PR

What type of PR is this

Proposed Changes

Proof Manifests

Checklist

Further Comments

Uh oh!

sonatype-lift bot commented Jul 15, 2023

Sonatype Lift is retiring

Uh oh!

codecov bot commented Jul 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

prady0t commented Jul 16, 2023

Uh oh!

realshuting left a comment

Choose a reason for hiding this comment

Uh oh!

JimBugwadia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

realshuting commented Jul 19, 2023

Uh oh!

welcome bot commented Jul 19, 2023

Congratulations! 🎉

Uh oh!

gcp-cherry-pick-bot bot commented Jul 19, 2023

Uh oh!

realshuting commented Jul 19, 2023

Uh oh!

prady0t commented Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

realshuting commented Jul 19, 2023

Uh oh!

prady0t commented Jul 19, 2023

Uh oh!

prady0t commented Jul 19, 2023

Uh oh!

realshuting commented Jul 20, 2023

Uh oh!

prady0t commented Jul 20, 2023

Uh oh!

prady0t commented Jul 20, 2023

Uh oh!

realshuting commented Jul 21, 2023

Uh oh!

prady0t commented Jul 21, 2023

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jul 16, 2023 •

edited

Loading

prady0t commented Jul 19, 2023 •

edited

Loading