Skip to content

Commit

Permalink
Trident Operator bugfixes
Browse files Browse the repository at this point in the history
This PR addresses the following bugs:

1. Fixes k8_version label to use major/minor version to ensure characters such as + are dropped to avoid any creation failure.
2. Fix for overriding kubeletDir after the CR creation by fixing the merging strategy.
3. Adds the ability to override K8s timeout
4. Ability to identify actual failures (e.g. image pull error) vs temporary failure (e.g. failure due to latency in pulling the image).
  • Loading branch information
rohit-arora-dev committed Jun 23, 2020
1 parent 53bfe7d commit 7d05ba7
Show file tree
Hide file tree
Showing 7 changed files with 233 additions and 210 deletions.
78 changes: 21 additions & 57 deletions cli/k8s_client/k8s_cli_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"k8s.io/api/policy/v1beta1"
v13 "k8s.io/api/rbac/v1"
v1beta12 "k8s.io/api/storage/v1beta1"
"k8s.io/apimachinery/pkg/types"
"os/exec"
"regexp"
"strings"
Expand Down Expand Up @@ -398,20 +399,8 @@ func (c *KubectlClient) DeleteDeployment(name, namespace string) error {

// PatchDeploymentByLabel patches a deployment object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchDeploymentByLabel(label string, patchBytes []byte) error {

cmdArgs := []string{"patch", "deployment", "-l", label, "--namespace", c.namespace, "-p", string(patchBytes), "--type", "strategic"}
out, err := exec.Command(c.cli, cmdArgs...).CombinedOutput()
if err != nil {
return fmt.Errorf("%s; %v", string(out), err)
}

log.WithFields(log.Fields{
"label": label,
"namespace": c.namespace,
}).Debug("Patched Kubernetes deployment.")

return nil
func (c *KubectlClient) PatchDeploymentByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, c.namespace, "deployment", "Deployment", patchBytes, patchType)
}

// GetServiceByLabel returns a service object matching the specified label if it is unique
Expand Down Expand Up @@ -517,21 +506,8 @@ func (c *KubectlClient) DeleteService(name, namespace string) error {

// PatchServiceByLabel patches a deployment object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchServiceByLabel(label string, patchBytes []byte) error {

cmdArgs := []string{"patch", "service", "-l", label, "--namespace", c.namespace, "-p", string(patchBytes),
"--type", "strategic"}
out, err := exec.Command(c.cli, cmdArgs...).CombinedOutput()
if err != nil {
return fmt.Errorf("%s; %v", string(out), err)
}

log.WithFields(log.Fields{
"label": label,
"namespace": c.namespace,
}).Debug("Patched Kubernetes service.")

return nil
func (c *KubectlClient) PatchServiceByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, c.namespace, "service", "Service", patchBytes, patchType)
}

// GetStatefulSetByLabel returns a statefulset object matching the specified label if it is unique
Expand Down Expand Up @@ -738,20 +714,8 @@ func (c *KubectlClient) DeleteDaemonSet(name, namespace string) error {

// PatchDaemonSetByLabel patches a DaemonSet object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchDaemonSetByLabel(label string, patchBytes []byte) error {

cmdArgs := []string{"patch", "daemonset", "-l", label, "--namespace", c.namespace, "-p", string(patchBytes), "--type", "strategic"}
out, err := exec.Command(c.cli, cmdArgs...).CombinedOutput()
if err != nil {
return fmt.Errorf("%s; %v", string(out), err)
}

log.WithFields(log.Fields{
"label": label,
"namespace": c.namespace,
}).Debug("Patched Kubernetes daemonset.")

return nil
func (c *KubectlClient) PatchDaemonSetByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, c.namespace, "daemonset", "Daemonset", patchBytes, patchType)
}

// GetConfigMapByLabel returns a configmap object matching the specified label if it is unique
Expand Down Expand Up @@ -1240,8 +1204,8 @@ func (c *KubectlClient) DeletePodSecurityPolicy(pspName string) error {

// PatchPodSecurityPolicyByLabel patches a pod security policy object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchPodSecurityPolicyByLabel(label string, patchBytes []byte) error {
return c.PatchObjectByLabel(label, "", "psp", "Pod Security Policy", patchBytes)
func (c *KubectlClient) PatchPodSecurityPolicyByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, "", "psp", "Pod Security Policy", patchBytes, patchType)
}

// GetServiceAccountByLabel returns a service account object matching the specified label if it is unique
Expand Down Expand Up @@ -1324,8 +1288,8 @@ func (c *KubectlClient) DeleteServiceAccount(name, namespace string) error {

// PatchServiceAccountByLabel patches a Service Account object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchServiceAccountByLabel(label string, patchBytes []byte) error {
return c.PatchObjectByLabel(label, c.namespace, "serviceaccount", "Service Account", patchBytes)
func (c *KubectlClient) PatchServiceAccountByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, c.namespace, "serviceaccount", "Service Account", patchBytes, patchType)
}

// GetClusterRoleByLabel returns a cluster role object matching the specified label if it is unique
Expand Down Expand Up @@ -1403,8 +1367,8 @@ func (c *KubectlClient) DeleteClusterRole(name string) error {

// PatchClusterRoleByLabel patches a Cluster Role object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchClusterRoleByLabel(label string, patchBytes []byte) error {
return c.PatchObjectByLabel(label, "", "clusterrole", "Cluster Role", patchBytes)
func (c *KubectlClient) PatchClusterRoleByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, "", "clusterrole", "Cluster Role", patchBytes, patchType)
}

// GetClusterRoleBindingByLabel returns a cluster role binding object matching the specified label if it is unique
Expand Down Expand Up @@ -1482,8 +1446,8 @@ func (c *KubectlClient) DeleteClusterRoleBinding(name string) error {

// PatchClusterRoleBindingByLabel patches a Cluster Role binding object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchClusterRoleBindingByLabel(label string, patchBytes []byte) error {
return c.PatchObjectByLabel(label, "", "clusterrolebinding", "Cluster Role Binding", patchBytes)
func (c *KubectlClient) PatchClusterRoleBindingByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, "", "clusterrolebinding", "Cluster Role Binding", patchBytes, patchType)
}

// GetCSIDriverByLabel returns a CSI driver object matching the specified label if it is unique
Expand Down Expand Up @@ -1561,8 +1525,8 @@ func (c *KubectlClient) DeleteCSIDriver(name string) error {

// PatchCSIDriverByLabel patches a deployment object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchCSIDriverByLabel(label string, patchBytes []byte) error {
return c.PatchObjectByLabel(label, "", "CSIDriver", "CSI Driver", patchBytes)
func (c *KubectlClient) PatchCSIDriverByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, "", "CSIDriver", "CSI Driver", patchBytes, patchType)
}

// GetSecretsByLabel returns all secret object matching specified label
Expand Down Expand Up @@ -1757,8 +1721,8 @@ func (c *KubectlClient) DeleteSecret(name, namespace string) error {

// PatchPodSecurityPolicyByLabel patches a pod security policy object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchSecretByLabel(label string, patchBytes []byte) error {
return c.PatchObjectByLabel(label, c.namespace, "secret", "Secret", patchBytes)
func (c *KubectlClient) PatchSecretByLabel(label string, patchBytes []byte, patchType types.PatchType) error {
return c.PatchObjectByLabel(label, c.namespace, "secret", "Secret", patchBytes, patchType)
}

// CheckNamespaceExists returns true if the specified namespace exists, false otherwise.
Expand Down Expand Up @@ -2190,9 +2154,9 @@ func (c *KubectlClient) DeleteObject(name, namespace, kind, kindName string) err

// PatchObjectByLabelAndNamespace patches an object matching the specified label
// in the namespace of the client.
func (c *KubectlClient) PatchObjectByLabel(label, namespace, kind, kindName string, patchBytes []byte) error {
func (c *KubectlClient) PatchObjectByLabel(label, namespace, kind, kindName string, patchBytes []byte, patchType types.PatchType) error {

cmdArgs := []string{"patch", kind, "-l", label, "-p", string(patchBytes), "--type", "merge"}
cmdArgs := []string{"patch", kind, "-l", label, "-p", string(patchBytes), "--type", string(patchType)}
if namespace != "" {
cmdArgs = append(cmdArgs, "--namespace", namespace)
}
Expand Down
56 changes: 28 additions & 28 deletions cli/k8s_client/k8s_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime/schema"
commontypes "k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/version"
"k8s.io/client-go/discovery"
"k8s.io/client-go/dynamic"
Expand Down Expand Up @@ -83,13 +83,13 @@ type Interface interface {
CheckDeploymentExistsByLabel(label string, allNamespaces bool) (bool, string, error)
DeleteDeploymentByLabel(label string) error
DeleteDeployment(name, namespace string) error
PatchDeploymentByLabel(label string, patchBytes []byte) error
PatchDeploymentByLabel(label string, patchBytes []byte, patchType types.PatchType) error
GetServiceByLabel(label string, allNamespaces bool) (*v1.Service, error)
GetServicesByLabel(label string, allNamespaces bool) ([]v1.Service, error)
CheckServiceExistsByLabel(label string, allNamespaces bool) (bool, string, error)
DeleteServiceByLabel(label string) error
DeleteService(name, namespace string) error
PatchServiceByLabel(label string, patchBytes []byte) error
PatchServiceByLabel(label string, patchBytes []byte, patchType types.PatchType) error
GetStatefulSetByLabel(label string, allNamespaces bool) (*appsv1.StatefulSet, error)
GetStatefulSetsByLabel(label string, allNamespaces bool) ([]appsv1.StatefulSet, error)
CheckStatefulSetExistsByLabel(label string, allNamespaces bool) (bool, string, error)
Expand All @@ -100,7 +100,7 @@ type Interface interface {
CheckDaemonSetExistsByLabel(label string, allNamespaces bool) (bool, string, error)
DeleteDaemonSetByLabel(label string) error
DeleteDaemonSet(name, namespace string) error
PatchDaemonSetByLabel(label string, patchBytes []byte) error
PatchDaemonSetByLabel(label string, patchBytes []byte, patchType types.PatchType) error
GetConfigMapByLabel(label string, allNamespaces bool) (*v1.ConfigMap, error)
GetConfigMapsByLabel(label string, allNamespaces bool) ([]v1.ConfigMap, error)
CheckConfigMapExistsByLabel(label string, allNamespaces bool) (bool, string, error)
Expand Down Expand Up @@ -128,31 +128,31 @@ type Interface interface {
CheckPodSecurityPolicyExistsByLabel(label string) (bool, string, error)
DeletePodSecurityPolicyByLabel(label string) error
DeletePodSecurityPolicy(pspName string) error
PatchPodSecurityPolicyByLabel(label string, patchBytes []byte) error
PatchPodSecurityPolicyByLabel(label string, patchBytes []byte, patchType types.PatchType) error
GetServiceAccountByLabel(label string, allNamespaces bool) (*v1.ServiceAccount, error)
GetServiceAccountsByLabel(label string, allNamespaces bool) ([]v1.ServiceAccount, error)
CheckServiceAccountExistsByLabel(label string, allNamespaces bool) (bool, string, error)
DeleteServiceAccountByLabel(label string) error
DeleteServiceAccount(name, namespace string) error
PatchServiceAccountByLabel(label string, patchBytes []byte) error
PatchServiceAccountByLabel(label string, patchBytes []byte, patchType types.PatchType) error
GetClusterRoleByLabel(label string) (*v13.ClusterRole, error)
GetClusterRolesByLabel(label string) ([]v13.ClusterRole, error)
CheckClusterRoleExistsByLabel(label string) (bool, string, error)
DeleteClusterRoleByLabel(label string) error
DeleteClusterRole(name string) error
PatchClusterRoleByLabel(label string, patchBytes []byte) error
PatchClusterRoleByLabel(label string, patchBytes []byte, patchType types.PatchType) error
GetClusterRoleBindingByLabel(label string) (*v13.ClusterRoleBinding, error)
GetClusterRoleBindingsByLabel(label string) ([]v13.ClusterRoleBinding, error)
CheckClusterRoleBindingExistsByLabel(label string) (bool, string, error)
DeleteClusterRoleBindingByLabel(label string) error
DeleteClusterRoleBinding(name string) error
PatchClusterRoleBindingByLabel(label string, patchBytes []byte) error
PatchClusterRoleBindingByLabel(label string, patchBytes []byte, patchType types.PatchType) error
GetCSIDriverByLabel(label string) (*v1beta12.CSIDriver, error)
GetCSIDriversByLabel(label string) ([]v1beta12.CSIDriver, error)
CheckCSIDriverExistsByLabel(label string) (bool, string, error)
DeleteCSIDriverByLabel(label string) error
DeleteCSIDriver(name string) error
PatchCSIDriverByLabel(label string, patchBytes []byte) error
PatchCSIDriverByLabel(label string, patchBytes []byte, patchType types.PatchType) error
CheckNamespaceExists(namespace string) (bool, error)
CreateSecret(secret *v1.Secret) (*v1.Secret, error)
UpdateSecret(secret *v1.Secret) (*v1.Secret, error)
Expand All @@ -164,7 +164,7 @@ type Interface interface {
DeleteSecretDefault(secretName string) error
DeleteSecretByLabel(label string) error
DeleteSecret(name, namespace string) error
PatchSecretByLabel(label string, patchBytes []byte) error
PatchSecretByLabel(label string, patchBytes []byte, patchType types.PatchType) error
CreateObjectByFile(filePath string) error
CreateObjectByYAML(yaml string) error
DeleteObjectByFile(filePath string, ignoreNotFound bool) error
Expand Down Expand Up @@ -484,15 +484,15 @@ func (k *KubeClient) DeleteDeployment(name, namespace string) error {

// PatchDeploymentByLabel patches a deployment object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchDeploymentByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchDeploymentByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

deployment, err := k.GetDeploymentByLabel(label, false)
if err != nil {
return err
}

if _, err = k.clientset.AppsV1().Deployments(k.namespace).Patch(ctx(), deployment.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down Expand Up @@ -601,15 +601,15 @@ func (k *KubeClient) DeleteService(name, namespace string) error {

// PatchServiceByLabel patches a deployment object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchServiceByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchServiceByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

service, err := k.GetServiceByLabel(label, false)
if err != nil {
return err
}

if _, err = k.clientset.CoreV1().Services(k.namespace).Patch(ctx(), service.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down Expand Up @@ -814,15 +814,15 @@ func (k *KubeClient) DeleteDaemonSet(name, namespace string) error {

// PatchDaemonSetByLabel patches a DaemonSet object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchDaemonSetByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchDaemonSetByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

daemonSet, err := k.GetDaemonSetByLabel(label, false)
if err != nil {
return err
}

if _, err = k.clientset.AppsV1().DaemonSets(k.namespace).Patch(ctx(), daemonSet.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down Expand Up @@ -1248,15 +1248,15 @@ func (k *KubeClient) DeletePodSecurityPolicy(pspName string) error {

// PatchPodSecurityPolicyByLabel patches a pod security policy object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchPodSecurityPolicyByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchPodSecurityPolicyByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

psp, err := k.GetPodSecurityPolicyByLabel(label)
if err != nil {
return err
}

if _, err = k.clientset.PolicyV1beta1().PodSecurityPolicies().Patch(ctx(), psp.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down Expand Up @@ -1366,15 +1366,15 @@ func (k *KubeClient) DeleteServiceAccount(name, namespace string) error {

// PatchServiceAccountByLabel patches a Service Account object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchServiceAccountByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchServiceAccountByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

serviceAccount, err := k.GetServiceAccountByLabel(label, false)
if err != nil {
return err
}

if _, err = k.clientset.CoreV1().ServiceAccounts(k.namespace).Patch(ctx(), serviceAccount.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down Expand Up @@ -1476,15 +1476,15 @@ func (k *KubeClient) DeleteClusterRole(name string) error {

// PatchClusterRoleByLabel patches a Cluster Role object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchClusterRoleByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchClusterRoleByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

clusterRole, err := k.GetClusterRoleByLabel(label)
if err != nil {
return err
}

if _, err = k.clientset.RbacV1().ClusterRoles().Patch(ctx(), clusterRole.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down Expand Up @@ -1586,15 +1586,15 @@ func (k *KubeClient) DeleteClusterRoleBinding(name string) error {

// PatchClusterRoleBindingByLabel patches a Cluster Role binding object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchClusterRoleBindingByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchClusterRoleBindingByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

clusterRoleBinding, err := k.GetClusterRoleBindingByLabel(label)
if err != nil {
return err
}

if _, err = k.clientset.RbacV1().ClusterRoleBindings().Patch(ctx(), clusterRoleBinding.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down Expand Up @@ -1695,15 +1695,15 @@ func (k *KubeClient) DeleteCSIDriver(name string) error {

// PatchCSIDriverByLabel patches a deployment object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchCSIDriverByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchCSIDriverByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

csiDriver, err := k.GetCSIDriverByLabel(label)
if err != nil {
return err
}

if _, err = k.clientset.StorageV1beta1().CSIDrivers().Patch(ctx(), csiDriver.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down Expand Up @@ -1856,15 +1856,15 @@ func (k *KubeClient) DeleteSecret(name, namespace string) error {

// PatchPodSecurityPolicyByLabel patches a pod security policy object matching the specified label
// in the namespace of the client.
func (k *KubeClient) PatchSecretByLabel(label string, patchBytes []byte) error {
func (k *KubeClient) PatchSecretByLabel(label string, patchBytes []byte, patchType types.PatchType) error {

secret, err := k.GetSecretByLabel(label, false)
if err != nil {
return err
}

if _, err = k.clientset.CoreV1().Secrets(k.namespace).Patch(ctx(), secret.Name,
commontypes.StrategicMergePatchType, patchBytes, patchOpts); err != nil {
patchType, patchBytes, patchOpts); err != nil {
return err
}

Expand Down
Loading

0 comments on commit 7d05ba7

Please sign in to comment.