-
Notifications
You must be signed in to change notification settings - Fork 128
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 loading of image translation CRDs #815
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 3 of 3 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @ivan4th)
pkg/manager/manager.go, line 64 at r1 (raw file):
// NewVirtletManager creates a new VirtletManager. func NewVirtletManager(config *v1.VirtletConfig, fdManager tapmanager.FDManager, clientCfg clientcmd.ClientConfig, diagSet *diag.Set) *VirtletManager { return &VirtletManager{config: config, fdManager: fdManager, diagSet: diagSet, clientCfg: clientCfg}
That's ok, but imo GetDefaultImageTranslator should return an error on nil clientCfg, instead of skipping crd source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @jellonek)
pkg/manager/manager.go, line 64 at r1 (raw file):
Previously, jellonek (Piotr Skamruk) wrote…
That's ok, but imo GetDefaultImageTranslator should return an error on nil clientCfg, instead of skipping crd source.
nil
clientCfg is actually used by integration tests that presently don't use CRD based image translation. The integration tests run w/o k8s yet they create an instance of VirtletManager
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 1 change requests, 0 of 1 approvals obtained
pkg/manager/manager.go, line 64 at r1 (raw file):
Previously, ivan4th (Ivan Shvedunov) wrote…
nil
clientCfg is actually used by integration tests that presently don't use CRD based image translation. The integration tests run w/o k8s yet they create an instance ofVirtletManager
.
Ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! 1 of 1 approvals obtained
This change is