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

Refactors and moves buildConfigs, imageStreams and some other resources #4261

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -271,7 +271,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 @@ -293,22 +293,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 @@ -318,7 +314,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