Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(seldon operator): "Deployment differs"-logs will never show #5141

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

caozhuozi
Copy link
Contributor

@caozhuozi caozhuozi commented Sep 27, 2023

What this PR does / why we need it:

The PR tries to fix #5140

Which issue(s) this PR fixes:

Fixes #5140

Special notes for your reviewer:

I think I already provided enough information on the issue page.
To emphasize, I mirrored some important conclusions here:

  1. an important fact about zap and logr (zapr): https://github.com/go-logr/zapr#increasing-verbosity
  2. the following code can reproduce the issue and back my findings:
    package main
    
    import (
       "go.uber.org/zap"
       zapf "sigs.k8s.io/controller-runtime/pkg/log/zap"
    )
    
    func main() {
    
      level := zap.DebugLevel
    
      atomicLevel := zap.NewAtomicLevelAt(level)
    
      logger := zapf.New(
    	  zapf.UseDevMode(false),
    	  zapf.Level(&atomicLevel),
      )
        
          // For DebugLevel, Only When `V` is set to 1, the log can be shown
      logger.V(5).Info("log sth")
    }
  3. Also note that when V is set to 1, for zap's InfoLevel, the log will not shown. I think this is the original design?

@seldondev
Copy link
Collaborator

Hi @caozhuozi. Thanks for your PR.

I'm waiting for a SeldonIO or todo member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the jenkins-x/lighthouse repository.

@caozhuozi caozhuozi force-pushed the fix/deployment-differ-log branch from cbe1270 to e403458 Compare September 27, 2023 12:39
@ukclivecox ukclivecox added the v1 label Oct 9, 2023
@ukclivecox ukclivecox merged commit 46ef14e into SeldonIO:master Oct 9, 2023
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Deployment differs"-logs will never show in seldon controller
3 participants