diff --git a/operator/utils/k8s/configmap.go b/operator/utils/k8s/configmap.go index 4a95a060ac..3e1926370f 100644 --- a/operator/utils/k8s/configmap.go +++ b/operator/utils/k8s/configmap.go @@ -22,7 +22,7 @@ type ConfigmapCreator struct { func NewConfigmapCreator(client kubernetes.Interface, logger logr.Logger, scheme *runtime.Scheme) *ConfigmapCreator { return &ConfigmapCreator{ clientset: client, - logger: logger, + logger: logger.WithName("ConfigMapCreator"), scheme: scheme, } } @@ -43,6 +43,7 @@ func (cc *ConfigmapCreator) CreateConfigmap(ctx context.Context, rawYaml []byte, // add ownership err = ctrl.SetControllerReference(owner, &cm, cc.scheme) if err != nil { + cc.logger.Error(err, "Failed to set controller reference on configmap") return err } diff --git a/operator/utils/k8s/crd.go b/operator/utils/k8s/crd.go index 2c5a092a50..9e3375fc68 100644 --- a/operator/utils/k8s/crd.go +++ b/operator/utils/k8s/crd.go @@ -24,7 +24,7 @@ func NewCrdCreator(ctx context.Context, apiExtensionsClient apiextensionsclient. return &CrdCreator{ apiExtensionsClient: apiExtensionsClient, discoveryClient: discoveryClient, - logger: logger, + logger: logger.WithName("CRDCreator"), ctx: ctx, } } @@ -43,6 +43,7 @@ func (cc *CrdCreator) createCRDV1beta1(rawYaml []byte) (*v1beta1.CustomResourceD crd := v1beta1.CustomResourceDefinition{} err := yaml.Unmarshal(rawYaml, &crd) if err != nil { + cc.logger.Error(err, "Failed to unmarshall v1beta1 CRD") return nil, err } client := cc.apiExtensionsClient.ApiextensionsV1beta1().CustomResourceDefinitions() @@ -53,6 +54,7 @@ func (cc *CrdCreator) createCRDV1(rawYaml []byte) (*extensionsv1.CustomResourceD crd := extensionsv1.CustomResourceDefinition{} err := yaml.Unmarshal(rawYaml, &crd) if err != nil { + cc.logger.Error(err, "Failed to unmarshall V1 CRD") return nil, err } client := cc.apiExtensionsClient.ApiextensionsV1().CustomResourceDefinitions() @@ -68,10 +70,12 @@ func (cc *CrdCreator) findOrCreateCRDV1beta1(rawYaml []byte) (v1.Object, error) cc.logger.Info("CRD v1beta1 not found - trying to create") crd, err = cc.createCRDV1beta1(rawYaml) if err != nil { + cc.logger.Error(err, "Failed to create v1beta1 CRD") return nil, err } cc.logger.Info("CRD v1beta1 created") } else { + cc.logger.Error(err, "Failed finding v1beta1 crd") return nil, err } } else { @@ -89,10 +93,12 @@ func (cc *CrdCreator) findOrCreateCRDV1(rawYaml []byte) (*extensionsv1.CustomRes cc.logger.Info("CRD V1 not found - trying to create") crd, err = cc.createCRDV1(rawYaml) if err != nil { + cc.logger.Error(err, "Failed to create v1 CRD") return nil, err } cc.logger.Info("CRD V1 created") } else { + cc.logger.Error(err, "Failed finding V1 CRD") return nil, err } } else { @@ -104,14 +110,17 @@ func (cc *CrdCreator) findOrCreateCRDV1(rawYaml []byte) (*extensionsv1.CustomRes func (cc *CrdCreator) findOrCreateCRD(rawYamlv1 []byte, rawYamlv1beta1 []byte) (v1.Object, error) { serverVersion, err := GetServerVersion(cc.discoveryClient, cc.logger) if err != nil { + cc.logger.Error(err, "Failed to get version from cluster") return nil, err } v, err := semver.NewVersion(serverVersion) if err != nil { + cc.logger.Error(err, "Failed to create semver Version") return nil, err } c, err := semver.NewConstraint(">= 1.18.0") if err != nil { + cc.logger.Error(err, "Failed applying constraint to check greater than 1.18.0 cluster") return nil, err } check := c.Check(v) diff --git a/operator/utils/k8s/initializer.go b/operator/utils/k8s/initializer.go index 476ff186c4..2c73cd964c 100644 --- a/operator/utils/k8s/initializer.go +++ b/operator/utils/k8s/initializer.go @@ -48,35 +48,42 @@ func InitializeOperator(ctx context.Context, config *rest.Config, namespace stri apiExtensionClient, err := apiextensionsclient.NewForConfig(config) if err != nil { + logger.Error(err, "Failed to create apiextensionsClient") return err } discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) if err != nil { + logger.Error(err, "Failed to create discoveryClient") return err } crdCreator := NewCrdCreator(ctx, apiExtensionClient, discoveryClient, logger) bytesV1Beta1, err := LoadBytesFromFile(ResourceFolder, CRDFilenameV1Beta1) if err != nil { + logger.Error(err, "Failed to find crd v1beta1", "resourcefolder", ResourceFolder, "filename", CRDFilenameV1Beta1) return err } bytesV1, err := LoadBytesFromFile(ResourceFolder, CRDFilenameV1) if err != nil { + logger.Error(err, "Failed to find crd v1", "resourcefolder", ResourceFolder, "filename", CRDFilenameV1) return err } crd, err := crdCreator.findOrCreateCRD(bytesV1, bytesV1Beta1) if err != nil { + logger.Error(err, "Failed to create CRD") return err } clientset, err := kubernetes.NewForConfig(config) if err != nil { + logger.Error(err, "Failed to create clientset") return err } dep, err := findMyDeployment(ctx, clientset, namespace) if err != nil { + logger.Error(err, "Failed to find deployment") return err } @@ -85,38 +92,34 @@ func InitializeOperator(ctx context.Context, config *rest.Config, namespace stri host2 := fmt.Sprintf("seldon-webhook-service.%s.svc", namespace) certs, err := certSetup([]string{host1, host2}) if err != nil { + logger.Error(err, "Failed to create certs") return err } // Create webhooks - wc, err := NewWebhookCreator(clientset, certs, logger, scheme) - if err != nil { - return err - } - - //Delete mutating webhook if existing - err = wc.DeleteMutatingWebhook(ctx) - if err != nil { - return err - } + wc := NewWebhookCreator(clientset, certs, logger, scheme) //Create/Update Validating Webhook bytes, err := LoadBytesFromFile(ResourceFolder, ValidatingWebhookFilename) if err != nil { + logger.Error(err, "Failed to find webhook file", "resourcefolder", ResourceFolder, "filename", ValidatingWebhookFilename) return err } err = wc.CreateValidatingWebhookConfigurationFromFile(ctx, bytes, namespace, crd, watchNamespace) if err != nil { + logger.Error(err, "Failed to create validating webhook") return err } //Create/Update Webhook Service bytes, err = LoadBytesFromFile(ResourceFolder, ServiceFilename) if err != nil { + logger.Error(err, "Failed to find webhook service", "resourcefolder", ResourceFolder, "filename", ServiceFilename) return err } err = wc.CreateWebhookServiceFromFile(ctx, bytes, namespace, dep) if err != nil { + logger.Error(err, "Failed to create webhook service") return err } @@ -124,39 +127,52 @@ func InitializeOperator(ctx context.Context, config *rest.Config, namespace stri cc := NewConfigmapCreator(clientset, logger, scheme) bytes, err = LoadBytesFromFile(ResourceFolder, ConfigMapFilename) if err != nil { + logger.Error(err, "Failed to find configmap", "resourcefolder", ResourceFolder, "filename", ConfigMapFilename) return err } err = cc.CreateConfigmap(ctx, bytes, namespace, dep) if err != nil { + logger.Error(err, "Failed to create webhook") return err } // Create cert files - createCertFiles(certs, logger) + err = createCertFiles(certs, logger) + if err != nil { + logger.Error(err, "Failed to create crts files") + return err + } return nil } func createCertFiles(certs *Cert, logger logr.Logger) error { //Save certs to filesystem - os.MkdirAll(CertsFolder, os.ModePerm) + err := os.MkdirAll(CertsFolder, os.ModePerm) + if err != nil { + logger.Error(err, "Failed to create folder", "folder", CertsFolder) + return err + } filename := fmt.Sprintf("%s/%s", CertsFolder, CertsTLSCa) logger.Info("Creating ", "filename", filename) - err := ioutil.WriteFile(filename, []byte(certs.caPEM), 0600) + err = ioutil.WriteFile(filename, []byte(certs.caPEM), 0600) if err != nil { + logger.Error(err, "failed to create cert file", "filename", filename) return err } filename = fmt.Sprintf("%s/%s", CertsFolder, CertsTLSKey) logger.Info("Creating ", "filename", filename) err = ioutil.WriteFile(filename, []byte(certs.privKeyPEM), 0600) if err != nil { + logger.Error(err, "failed to create cert file", "filename", filename) return err } filename = fmt.Sprintf("%s/%s", CertsFolder, CertsTLSCrt) logger.Info("Creating ", "filename", filename) err = ioutil.WriteFile(filename, []byte(certs.certificatePEM), 0600) if err != nil { + logger.Error(err, "failed to create cert file", "filename", filename) return err } diff --git a/operator/utils/k8s/utils.go b/operator/utils/k8s/utils.go index ee5cdb5bcd..50cba5cbbc 100644 --- a/operator/utils/k8s/utils.go +++ b/operator/utils/k8s/utils.go @@ -14,6 +14,7 @@ const DefaultMinorVersion = 18 func GetServerVersion(client discovery.DiscoveryInterface, logger logr.Logger) (string, error) { serverVersion, err := client.ServerVersion() if err != nil { + logger.Error(err, "Failed to get server version") return "", err } logger.Info("Server version", "Major", serverVersion.Major, "Minor", serverVersion.Minor) diff --git a/operator/utils/k8s/webhook.go b/operator/utils/k8s/webhook.go index 07da4f938a..06457ee4df 100644 --- a/operator/utils/k8s/webhook.go +++ b/operator/utils/k8s/webhook.go @@ -4,7 +4,7 @@ import ( "context" "github.com/ghodss/yaml" "github.com/go-logr/logr" - "k8s.io/api/admissionregistration/v1beta1" + adv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -23,48 +23,35 @@ type WebhookCreator struct { scheme *runtime.Scheme } -func NewWebhookCreator(client kubernetes.Interface, certs *Cert, logger logr.Logger, scheme *runtime.Scheme) (*WebhookCreator, error) { +func NewWebhookCreator(client kubernetes.Interface, certs *Cert, logger logr.Logger, scheme *runtime.Scheme) *WebhookCreator { return &WebhookCreator{ clientset: client, certs: certs, - logger: logger, + logger: logger.WithName("WebhookCreator"), scheme: scheme, - }, nil -} - -func (wc *WebhookCreator) DeleteMutatingWebhook(ctx context.Context) error { - client := wc.clientset.AdmissionregistrationV1beta1().MutatingWebhookConfigurations() - - // Try to delete clusterwide webhook config if available (older versions of seldon core) - _, err := client.Get(ctx, MutatingWebhookName, v1.GetOptions{}) - if err != nil && errors.IsNotFound(err) { - wc.logger.Info("existing clusterwide mwc not found", "name", MutatingWebhookName) - } else { - client.Delete(ctx, MutatingWebhookName, v1.DeleteOptions{}) - if err != nil { - return err - } - wc.logger.Info("Deleted clusterwide mwc", "name", MutatingWebhookName) } - - return nil } func (wc *WebhookCreator) CreateValidatingWebhookConfigurationFromFile(ctx context.Context, rawYaml []byte, namespace string, owner v1.Object, watchNamespace bool) error { - vwc := v1beta1.ValidatingWebhookConfiguration{} + vwc := adv1.ValidatingWebhookConfiguration{} err := yaml.Unmarshal(rawYaml, &vwc) if err != nil { + wc.logger.Error(err, "Failed to unmarshall validating webhook") return err } // create or update via client - client := wc.clientset.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations() + client := wc.clientset.AdmissionregistrationV1().ValidatingWebhookConfigurations() if watchNamespace { // Try to delete clusterwide webhook config if available _, err := client.Get(ctx, vwc.Name, v1.GetOptions{}) - if err != nil && errors.IsNotFound(err) { - wc.logger.Info("existing clusterwide vwc not found", "name", vwc.Name) + if err != nil { + if errors.IsNotFound(err) { + wc.logger.Info("existing clusterwide vwc not found", "name", vwc.Name) + } else { + wc.logger.Error(err, "Ignoring error trying to get existing validating webhook") + } } else { client.Delete(ctx, vwc.Name, v1.DeleteOptions{}) if err != nil { @@ -91,6 +78,7 @@ func (wc *WebhookCreator) CreateValidatingWebhookConfigurationFromFile(ctx conte // add ownership err = ctrl.SetControllerReference(owner, &vwc, wc.scheme) if err != nil { + wc.logger.Error(err, "Failed to set owner reference on validating webhook") return err } @@ -98,10 +86,16 @@ func (wc *WebhookCreator) CreateValidatingWebhookConfigurationFromFile(ctx conte if err != nil && errors.IsNotFound(err) { wc.logger.Info("Creating validating webhook") _, err = client.Create(ctx, &vwc, v1.CreateOptions{}) + if err != nil { + wc.logger.Error(err, "Failed to create validating webhook") + } } else if err == nil { wc.logger.Info("Updating validating webhook") found.Webhooks = vwc.Webhooks _, err = client.Update(ctx, found, v1.UpdateOptions{}) + if err != nil { + wc.logger.Error(err, "Failed to update validating webhook") + } return err } return err @@ -135,9 +129,12 @@ func (wc *WebhookCreator) CreateWebhookServiceFromFile(ctx context.Context, rawY if err != nil && errors.IsNotFound(err) { wc.logger.Info("Creating webhook svc") _, err = client.Create(ctx, &svc, v1.CreateOptions{}) + if err != nil { + wc.logger.Error(err, "Failed to update webhook svc") + } } else if err == nil { wc.logger.Info("Webhook svc exists won't update - need a patch in future") - return err + return nil } return err } diff --git a/operator/utils/k8s/webhook_test.go b/operator/utils/k8s/webhook_test.go index a21839ebbe..899333f63a 100644 --- a/operator/utils/k8s/webhook_test.go +++ b/operator/utils/k8s/webhook_test.go @@ -110,11 +110,10 @@ func TestValidatingWebhookCreate(t *testing.T) { hosts := []string{"seldon-webhook-service.seldon-system", "seldon-webhook-service.seldon-system.svc"} certs, err := certSetup(hosts) g.Expect(err).To(BeNil()) - wc, err := NewWebhookCreator(client, certs, ctrl.Log, scheme) - g.Expect(err).To(BeNil()) + wc := NewWebhookCreator(client, certs, ctrl.Log, scheme) err = wc.CreateValidatingWebhookConfigurationFromFile(context.TODO(), bytes, TestNamespace, crd, false) g.Expect(err).To(BeNil()) - _, err = client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Get(context.TODO(), "seldon-validating-webhook", metav1.GetOptions{}) + _, err = client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Get(context.TODO(), "seldon-validating-webhook", metav1.GetOptions{}) g.Expect(err).To(BeNil()) } @@ -130,11 +129,10 @@ func TestValidatingWebhookCreateNamespaced(t *testing.T) { hosts := []string{"seldon-webhook-service.seldon-system", "seldon-webhook-service.seldon-system.svc"} certs, err := certSetup(hosts) g.Expect(err).To(BeNil()) - wc, err := NewWebhookCreator(client, certs, ctrl.Log, scheme) - g.Expect(err).To(BeNil()) + wc := NewWebhookCreator(client, certs, ctrl.Log, scheme) err = wc.CreateValidatingWebhookConfigurationFromFile(context.TODO(), bytes, TestNamespace, crd, true) g.Expect(err).To(BeNil()) - _, err = client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Get(context.TODO(), "seldon-validating-webhook-"+TestNamespace, metav1.GetOptions{}) + _, err = client.AdmissionregistrationV1().ValidatingWebhookConfigurations().Get(context.TODO(), "seldon-validating-webhook-"+TestNamespace, metav1.GetOptions{}) g.Expect(err).To(BeNil()) } @@ -149,8 +147,7 @@ func TestWebHookSvcCreate(t *testing.T) { hosts := []string{"seldon-webhook-service.seldon-system", "seldon-webhook-service.seldon-system.svc"} certs, err := certSetup(hosts) g.Expect(err).To(BeNil()) - wc, err := NewWebhookCreator(client, certs, ctrl.Log, scheme) - g.Expect(err).To(BeNil()) + wc := NewWebhookCreator(client, certs, ctrl.Log, scheme) err = wc.CreateWebhookServiceFromFile(context.TODO(), bytes, TestNamespace, dep) g.Expect(err).To(BeNil()) }