Skip to content

Commit

Permalink
Refactors and moves buildConfigs and imageStreams related code into n…
Browse files Browse the repository at this point in the history
…ew files. (#4261)

It also refactors a few other resources like deployment etc.

Signed-off-by: mik-dass <mrinald7@gmail.com>
  • Loading branch information
mik-dass committed Dec 1, 2020
1 parent f8f8f1f commit cdfa545
Show file tree
Hide file tree
Showing 16 changed files with 2,702 additions and 2,646 deletions.
4 changes: 2 additions & 2 deletions pkg/catalog/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func getDefaultBuilderImages(client *occlient.Client) ([]ComponentType, error) {
currentNamespace := client.GetCurrentProjectName()

// Fetch imagestreams from default openshift namespace
openshiftNSImageStreams, openshiftNSISFetchError := client.GetImageStreams(occlient.OpenShiftNameSpace)
openshiftNSImageStreams, openshiftNSISFetchError := client.ListImageStreams(occlient.OpenShiftNameSpace)
if openshiftNSISFetchError != nil {
// Tolerate the error as it might only be a partial failure
// We may get the imagestreams from other Namespaces
Expand All @@ -397,7 +397,7 @@ func getDefaultBuilderImages(client *occlient.Client) ([]ComponentType, error) {
}

// Fetch imagestreams from current namespace
currentNSImageStreams, currentNSISFetchError := client.GetImageStreams(currentNamespace)
currentNSImageStreams, currentNSISFetchError := client.ListImageStreams(currentNamespace)
// If failure to fetch imagestreams from current namespace, log the failure for debugging purposes
if currentNSISFetchError != nil {
// Tolerate the error as it is totally a valid scenario to not have any imagestreams in current namespace
Expand Down
2 changes: 1 addition & 1 deletion pkg/component/component.go
Original file line number Diff line number Diff line change
Expand Up @@ -1268,7 +1268,7 @@ func Update(client *occlient.Client, componentConfig config.LocalConfigInfo, new

// we choose the env variables in the config over the one present in the DC
// so the local config is reflected on the cluster
evl, err := occlient.GetInputEnvVarsFromStrings(envVarsList.ToStringSlice())
evl, err := kclient.GetInputEnvVarsFromStrings(envVarsList.ToStringSlice())
if err != nil {
return err
}
Expand Down
16 changes: 6 additions & 10 deletions pkg/kclient/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func (c *Client) LinkSecret(secretName, componentName, applicationName string) e
return fmt.Sprintf(`[{ "op": "add", "path": "/spec/template/spec/containers/0/envFrom", "value": [{"secretRef": {"name": "%s"}}] }]`, secretName), nil
}

return c.patchDeploymentOfComponent(componentName, applicationName, deploymentPatchProvider)
return c.patchDeployment(componentName, deploymentPatchProvider)
}

// UnlinkSecret unlinks a secret to the Deployment of a component
Expand All @@ -269,22 +269,18 @@ func (c *Client) UnlinkSecret(secretName, componentName, applicationName string)
return fmt.Sprintf(`[{"op": "remove", "path": "/spec/template/spec/containers/0/envFrom/%d"}]`, indexForRemoval), nil
}

return c.patchDeploymentOfComponent(componentName, applicationName, deploymentPatchProvider)
return c.patchDeployment(componentName, deploymentPatchProvider)
}

// this function will look up the appropriate DC, and execute the specified patch
// the whole point of using patch is to avoid race conditions where we try to update
// deployment while it's being simultaneously updated from another source (for example Kubernetes itself)
// this will result in the triggering of a redeployment
func (c *Client) patchDeploymentOfComponent(componentName, applicationName string, deploymentPatchProvider deploymentPatchProvider) error {
// deploymentName, err := util.NamespaceOpenShiftObject(componentName, applicationName)
// if err != nil {
// return err
// }
func (c *Client) patchDeployment(deploymentName string, deploymentPatchProvider deploymentPatchProvider) error {

deployment, err := c.KubeClient.AppsV1().Deployments(c.Namespace).Get(componentName, metav1.GetOptions{})
deployment, err := c.GetDeploymentByName(deploymentName)
if err != nil {
return errors.Wrapf(err, "Unable to locate Deployment for component %s of application %s", componentName, applicationName)
return errors.Wrapf(err, "Unable to locate Deployment %s", deploymentName)
}

if deploymentPatchProvider != nil {
Expand All @@ -294,7 +290,7 @@ func (c *Client) patchDeploymentOfComponent(componentName, applicationName strin
}

// patch the Deployment with the secret
_, err = c.KubeClient.AppsV1().Deployments(c.Namespace).Patch(componentName, types.JSONPatchType, []byte(patch))
_, err = c.KubeClient.AppsV1().Deployments(c.Namespace).Patch(deploymentName, types.JSONPatchType, []byte(patch))
if err != nil {
return errors.Wrapf(err, "Deployment not patched %s", deployment.Name)
}
Expand Down
33 changes: 33 additions & 0 deletions pkg/kclient/utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package kclient

import (
"strings"

"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
)

// GetInputEnvVarsFromStrings generates corev1.EnvVar values from the array of string key=value pairs
// envVars is the array containing the key=value pairs
func GetInputEnvVarsFromStrings(envVars []string) ([]corev1.EnvVar, error) {
var inputEnvVars []corev1.EnvVar
var keys = make(map[string]int)
for _, env := range envVars {
splits := strings.SplitN(env, "=", 2)
if len(splits) < 2 {
return nil, errors.New("invalid syntax for env, please specify a VariableName=Value pair")
}
_, ok := keys[splits[0]]
if ok {
return nil, errors.Errorf("multiple values found for VariableName: %s", splits[0])
}

keys[splits[0]] = 1

inputEnvVars = append(inputEnvVars, corev1.EnvVar{
Name: splits[0],
Value: splits[1],
})
}
return inputEnvVars, nil
}
103 changes: 103 additions & 0 deletions pkg/kclient/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package kclient

import (
corev1 "k8s.io/api/core/v1"
"reflect"
"testing"
)

func TestGetInputEnvVarsFromStrings(t *testing.T) {
tests := []struct {
name string
envVars []string
wantedEnvVars []corev1.EnvVar
wantErr bool
}{
{
name: "Test case 1: with valid two key value pairs",
envVars: []string{"key=value", "key1=value1"},
wantedEnvVars: []corev1.EnvVar{
{
Name: "key",
Value: "value",
},
{
Name: "key1",
Value: "value1",
},
},
wantErr: false,
},
{
name: "Test case 2: one env var with missing value",
envVars: []string{"key=value", "key1="},
wantedEnvVars: []corev1.EnvVar{
{
Name: "key",
Value: "value",
},
{
Name: "key1",
Value: "",
},
},
wantErr: false,
},
{
name: "Test case 3: one env var with no value and no =",
envVars: []string{"key=value", "key1"},
wantErr: true,
},
{
name: "Test case 4: one env var with multiple values",
envVars: []string{"key=value", "key1=value1=value2"},
wantedEnvVars: []corev1.EnvVar{
{
Name: "key",
Value: "value",
},
{
Name: "key1",
Value: "value1=value2",
},
},
wantErr: false,
},
{
name: "Test case 5: two env var with same key",
envVars: []string{"key=value", "key=value1"},
wantErr: true,
},
{
name: "Test case 6: one env var with base64 encoded value",
envVars: []string{"key=value", "key1=SSd2ZSBnb3QgYSBsb3ZlbHkgYnVuY2ggb2YgY29jb251dHMhCg=="},
wantedEnvVars: []corev1.EnvVar{
{
Name: "key",
Value: "value",
},
{
Name: "key1",
Value: "SSd2ZSBnb3QgYSBsb3ZlbHkgYnVuY2ggb2YgY29jb251dHMhCg==",
},
},
wantErr: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
envVars, err := GetInputEnvVarsFromStrings(tt.envVars)

if err == nil && !tt.wantErr {
if !reflect.DeepEqual(tt.wantedEnvVars, envVars) {
t.Errorf("corev1.Env values are not matching with expected values, expected: %v, got %v", tt.wantedEnvVars, envVars)
}
} else if err == nil && tt.wantErr {
t.Error("error was expected, but no error was returned")
} else if err != nil && !tt.wantErr {
t.Errorf("test failed, no error was expected, but got unexpected error: %s", err)
}
})
}
}
Loading

0 comments on commit cdfa545

Please sign in to comment.