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.

Sign up for GitHub

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

Jump to bottom

Use specific client set for operations #105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use specific client set for operations #105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion pkg/utils/builders.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func BuildingAndLaunchJob(experiment *ExperimentDetails, clients ClientSets) err

// launchJob spawn a kubernetes Job using the job Object received.
func (experiment *ExperimentDetails) launchJob(job *batchv1.Job, clients ClientSets) error {
_, err := clients.KubeClient.BatchV1().Jobs(experiment.Namespace).Create(job)
_, err := clients.KubeClientExperiment.BatchV1().Jobs(experiment.Namespace).Create(job)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just trying to understand which config is used for which purpose.

  • kubeconfig & KubeClientExperiment for ChaosEngine, ChaosExperiment, ChaosRunner, ChaosOperator
  • litmuskubeconfig & k8sClientSetLitmus for TargetApplication, ExperimentJob/Pod, ChaosResult
    right? if this is the case, would you like to use the clients.KubeClientExperiment to get the chaosresult verdict here?
    .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I was thinking having the chausresult used by kubeconfig. Do you think it should be used by litmuskubeconfig?

if err != nil {
return err
}
Expand Down
55 changes: 42 additions & 13 deletions pkg/utils/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ type VolumeOpts struct {

// ClientSets is a collection of clientSets needed
type ClientSets struct {
KubeClient *kubernetes.Clientset
LitmusClient *clientV1alpha1.Clientset
KubeClient *kubernetes.Clientset
LitmusClient *clientV1alpha1.Clientset
KubeClientExperiment *kubernetes.Clientset
LitmusClientExperiment *clientV1alpha1.Clientset
}

// Recorder is collection of resources needed to record events for chaos-runner
Expand Down Expand Up @@ -95,40 +97,67 @@ const (

// GenerateClientSetFromKubeConfig will generation both ClientSets (k8s, and Litmus)
func (clientSets *ClientSets) GenerateClientSetFromKubeConfig() error {
config, err := getKubeConfig()
configExperiment, configLitmus, err := getKubeConfig()
if err != nil {
return err
}
k8sClientSet, err := k8s.GenerateK8sClientSet(config)

k8sClientSetLitmus, err := k8s.GenerateK8sClientSet(configLitmus)
if err != nil {
return err
}
litmusClientSet, err := litmus.GenerateLitmusClientSet(config)
litmusClientSetLitmus, err := litmus.GenerateLitmusClientSet(configLitmus)
if err != nil {
return err
}

k8sClientSetExperiment, err := k8s.GenerateK8sClientSet(configExperiment)
if err != nil {
return err
}
clientSets.KubeClient = k8sClientSet
clientSets.LitmusClient = litmusClientSet
litmusClientSetExperiment, err := litmus.GenerateLitmusClientSet(configExperiment)
if err != nil {
return err
}

clientSets.KubeClient = k8sClientSetLitmus
clientSets.LitmusClient = litmusClientSetLitmus
clientSets.KubeClientExperiment = k8sClientSetExperiment
clientSets.LitmusClientExperiment = litmusClientSetExperiment

return nil
}

// Generate

// getKubeConfig setup the config for access cluster resource
func getKubeConfig() (*rest.Config, error) {
func getKubeConfig() (*rest.Config, *rest.Config, error) {
kubeconfig := flag.String("kubeconfig", "", "absolute path to the kubeconfig file")
litmuskubeconfig := flag.String("litmuskubeconfig", "", "absolute path to the kubeconfig file")
Copy link
Member

@ksatchit ksatchit Oct 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we can also distinguish the usage/help message to specify:

litmuskubeconfig - "absolute path to the kubeconfig file of target cluster where experiment job is launched"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref: #105 (comment)

flag.Parse()
// Use in-cluster config if kubeconfig path is specified
if *litmuskubeconfig == "" {
configLitmus, err := rest.InClusterConfig()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not be configExperiment ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually no, whatever is specified by litmuskubeconfig parameter is then defined in configLitmus variable. And whatever is specified in kubeconfig parameter is then defined in configExperiment variable.

Copy link
Member

@ksatchit ksatchit Oct 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok got it. Could we name this better. Can we say litmusControlPlaneKubeConfig as the one which deals w/ chaos-operator/runner/experimentCR/engineCR & targetKubeConfig as the one that deals w/ the target application & also the experiment job/helper pod & chaosresult resources (note that chaosresult CR is created by the experiment and thereby will reside in the target cluster) .. the logic to read chaosResults and patch to enginine may need to be looked at

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

if err != nil {
return nil, configLitmus, err
}
}
configLitmus, err := clientcmd.BuildConfigFromFlags("", *litmuskubeconfig)
if err != nil {
return nil, configLitmus, err
}

// Use in-cluster config if kubeconfig path is specified
if *kubeconfig == "" {
config, err := rest.InClusterConfig()
configExperiment, err := rest.InClusterConfig()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this, the configLitmus?

if err != nil {
return config, err
return configExperiment, configLitmus, err
}
}
config, err := clientcmd.BuildConfigFromFlags("", *kubeconfig)
configExperiment, err := clientcmd.BuildConfigFromFlags("", *kubeconfig)
if err != nil {
return config, err
return configExperiment, configLitmus, err
}
return config, err

return configExperiment, configLitmus, err
}
2 changes: 1 addition & 1 deletion pkg/utils/watchChaosContainer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

// GetChaosPod gets the chaos experiment pod object launched by the runner
func GetChaosPod(expDetails *ExperimentDetails, clients ClientSets) (*corev1.Pod, error) {
chaosPodList, err := clients.KubeClient.CoreV1().Pods(expDetails.Namespace).List(metav1.ListOptions{LabelSelector: "job-name=" + expDetails.JobName})
chaosPodList, err := clients.KubeClientExperiment.CoreV1().Pods(expDetails.Namespace).List(metav1.ListOptions{LabelSelector: "job-name=" + expDetails.JobName})
if err != nil || len(chaosPodList.Items) == 0 {
return nil, errors.Errorf("Unable to get the chaos pod, error: %v", err)
} else if len(chaosPodList.Items) > 1 {
Expand Down