Review snapshots and daemonsets #201
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
gianrubio
commented
May 14, 2018
gianrubio
commented
- Deploy es daemonset in the same namespace of the crd
- Delete daemonset object before a crd deletiong
- Set history size in snapshots cronjob
…e namespace of the crd, delete daemonset object before a crd deletiong
danisla
reviewed
May 24, 2018
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| err = c.k8sclient.CreateNodeInitDaemonset("default") | ||
| return c.k8sclient.CreateNodeInitDaemonset(crd.ObjectMeta.Namespace) |
There was a problem hiding this comment.
Getting this error:
time="2018-05-24T00:39:25Z" level=info msg="Daemonset not found, creating..."
time="2018-05-24T00:39:25Z" level=error msg="Error in init(): an empty namespace may not be set during creation"
CRD does not have a namespace as it is cluster-wide.
Would this work to just create it in the same namespace as the controller?
ns, _ := os.LookupEnv("NAMESPACE")
return c.k8sclient.CreateNodeInitDaemonset(ns)
danisla
reviewed
May 24, 2018
| // DeleteNodeInitDaemonset delete the node init daemonset | ||
| func (k *K8sutil) DeleteNodeInitDaemonset(namespace string) error { | ||
|
|
||
| ds, err := k.Kclient.ExtensionsV1beta1().DaemonSets(namespace).Get(esOperatorSysctlName, metav1.GetOptions{}) |
There was a problem hiding this comment.
This should be the .Delete() function, not the.Get().
|
Hey @gianrubio do you have time to look at the comments from @danisla? |
danisla
reviewed
Jun 28, 2018
| @@ -221,7 +221,7 @@ func (k *K8sutil) CreateKubernetesCustomResourceDefinition() error { | |||
| logrus.Infof("SKIPPING: already exists %#v\n", crd.ObjectMeta.Name) | |||
| } | |||
|
|
|||
| return nil | |||
| return crd, err | |||
There was a problem hiding this comment.
This should be: return crd, nil otherwise on first install, if the CRD doesn't exist, it will return the value of err from the first test. This breaks the controller init() function.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.