feat: Runtime variables and system config change reload #352

Vafilor · 2020-06-16T01:58:09Z

What this PR does:

Updates the workspace templates so that runtime variables like the DOMAIN are injected when the workspace is is created instead of when the workspace template is created.

This also updates the server so that it reloads data upon a detected configuration change, so variables like DOMAIN are always up to date.

Which issue(s) this PR fixes:

Fixes #348

Special notes for your reviewer:

For the runtime variables, there are 3 main places of interest.

When we create a template for viewing, as when we generate a dynamic template while creating/editing a workspace template. In this situation, we need to inject runtime variables for things like the Machine Type dropdown.
Whenever we get a Workspace Template, we need to modify the manifest to include the variables.
When we create the Workflow Execution for a Workspace. The Argo WorkflowTemplate needs to have the appropriate variables injected into it.

NOTE:
If you attempt to view the YAML in the workflow execution, it will not have the injected variables as it gets the Workflow Execution, and we do not know to inject the runtime variables there. We might want to eventually add a method to get the workflow execution yaml for a Workspace to get around this.

… updates the server accordingly.

… as it depends on config.

Vafilor · 2020-06-16T01:58:46Z

main.go

 	}
+
+	go Run(client, "onepanel", stopCh, func(configMap *corev1.ConfigMap) error {
+		log.Printf("Configmap changed")


We can get rid of this, it's more for debugging.

Vafilor · 2020-06-16T01:58:54Z

main.go

+			log.Fatalf("Failed to serve RPC server: %v", err)
+		}
+
+		log.Infof("GRPC finished")


We can get rid of this, it's more for debugging.

pkg/workspace_template.go

rushtehrani · 2020-06-17T19:22:40Z

@Vafilor getting this error when trying to create a Workspace:

Vafilor · 2020-06-17T19:30:07Z

@Vafilor getting this error when trying to create a Workspace:

@rushtehrani Added a migration. Please pull and try again.

rushtehrani · 2020-06-17T19:48:09Z

@rushtehrani Added a migration. Please pull and try again.

That did it

rushtehrani · 2020-06-17T19:52:47Z

@Vafilor Another issue, I think a merge/rebase has gone wrong:

Pod "rbac-test-0" is invalid: spec.containers[0].volumeMounts[1].name: Not found: "sys-dshm"

Vafilor · 2020-06-17T19:54:03Z

@Vafilor Another issue, I think a merge/rebase has gone wrong:
Pod "rbac-test-0" is invalid: spec.containers[0].volumeMounts[1].name: Not found: "sys-dshm"

Taking a look.

rushtehrani · 2020-06-17T19:55:57Z

I think it's the missing volume

Vafilor · 2020-06-17T19:59:14Z

I think it's the missing volume

That's most likely it - pushed up a fix.

rushtehrani · 2020-06-17T21:15:36Z

pkg/workflow_execution.go

 		return nil, util.NewUserError(codes.NotFound, "Error with getting workflow template.")
 	}

+	if runtimeVars != nil && workflowTemplate.ArgoWorkflowTemplate != nil {


I don't think this section should be here. It's unique to Workspaces only. Other calls to CreateWorkflowExecution may never have "stateful-set-resource" or VirtualService. Can we inject these into the template before it's passed in here?

@rushtehrani I agree, we need to separate the concerns here.

The code above gets the workflow template and that also gets the k8s argo template. At the moment, we can't modify it beforehand because it gets it in this function.

We could change the function so that we pass in the workflow template with the argo contents pre-loaded.
That way it does less work and is more configurable.

What do you think?

@Vafilor parameters are showing and configuration reload is working. Is this the only change that remains?

@rushtehrani Yes, I should have it ready by tomorrow morning.

rushtehrani · 2020-06-17T21:22:37Z

pkg/workspace.go

+		return nil, err
+	}
+
+	_, err = c.CreateWorkflowExecution(namespace, &WorkflowExecution{


@Vafilor in reference to this comment. Ideally this we want to make the override before they're passed into CreateWorkflowExecution here.

rushtehrani · 2020-06-17T21:45:35Z

@Vafilor another issue I ran into is that the nodepool changes don't update after 2 or 3 changes, seems like the watcher stops working after a while.

Vafilor · 2020-06-17T21:46:07Z

@Vafilor another issue I ran into is that the nodepool changes don't update after 2 or 3 changes, seems like the watcher stops working after a while.

That's weird. I'll take a look.

rushtehrani · 2020-06-18T00:15:33Z

@Vafilor another issue is that now sys-nodepool and other runtime params aren't showing in Workspace details.

rushtehrani · 2020-06-18T01:01:39Z

@Vafilor nodepool values not updating when config is updated is still an issue. I'm testing by updating applicationNodePoolOptions values in system configmap and I don't see the changes in the UI.

… updates the server accordingly.

… as it depends on config.

…onepanelio/core into feat/onepanelio.core-348.system.updates

rushtehrani · 2020-06-18T20:58:12Z

pkg/config.go

+}
+
+// OnepanelDomain gets the ONEPANEL_DOMAIN value, or nil.
+func (s SystemConfig) OnepanelDomain() *string {


Let's just call this Domain. I think we should also have FQDN here too.

@rushtehrani Makes sense, and sure thing. I'm slowly adding them as I encounter them in the code.

Vafilor added 5 commits June 11, 2020 20:19

poc: proof of concept for a server that detects configmap changes and…

64a078f

… updates the server accordingly.

update: workspace url is no longer stored in db but is auto generated…

9c356df

… as it depends on config.

fix: removed reference to workspace.url

9f6a1be

fix: removed db reference to workspace url

a983e26

update: added logic to inject runtime variables in Workspace templates

d56b728

Vafilor requested a review from rushtehrani June 16, 2020 01:58

Vafilor changed the base branch from master to dev June 16, 2020 01:58

Vafilor commented Jun 16, 2020

View reviewed changes

Vafilor added 3 commits June 16, 2020 10:30

clean: code cleanup and minor documentation

ba72b14

Merge branch 'dev' into feat/onepanelio.core-348.system.updates

3074622

fix: parameter name

56777b4

rushtehrani reviewed Jun 17, 2020

View reviewed changes

pkg/workspace_template.go Outdated Show resolved Hide resolved

workspaceSpec => spec

504c4bf

rushtehrani added this to the v0.11.0 milestone Jun 17, 2020

fix: added missing migration

c91efc7

fix: added missing volume

b6c67dd

rushtehrani reviewed Jun 17, 2020

View reviewed changes

fix: issue where watch was started multiple times.

63011e1

Vafilor added 13 commits June 17, 2020 19:56

test: locks instead of channels

7df9ac6

fix: fixed issue where channels were not working for restarting server.

fddf2d6

poc: proof of concept for a server that detects configmap changes and…

2434864

… updates the server accordingly.

update: workspace url is no longer stored in db but is auto generated…

72d6892

… as it depends on config.

fix: removed reference to workspace.url

0e2eb15

fix: removed db reference to workspace url

8c8d4e2

update: added logic to inject runtime variables in Workspace templates

8684e44

clean: code cleanup and minor documentation

6fe23c7

fix: added missing migration

efffe1f

fix: issue where watch was started multiple times.

aaf6d31

test: locks instead of channels

9e236aa

fix: fixed issue where channels were not working for restarting server.

d823a95

Merge branch 'feat/onepanelio.core-348.system.updates' of github.com:…

e1ade14

…onepanelio/core into feat/onepanelio.core-348.system.updates

rushtehrani changed the base branch from dev to master June 18, 2020 20:56

rushtehrani changed the base branch from master to dev June 18, 2020 20:56

rushtehrani reviewed Jun 18, 2020

View reviewed changes

Vafilor added 2 commits June 18, 2020 15:22

update: config updates and related usage updates.

ce9fd4b

fix: issue where getting workspace did not have the runtime parameters

49c1c6a

rushtehrani merged commit 5fe3ef9 into dev Jun 19, 2020

rushtehrani deleted the feat/onepanelio.core-348.system.updates branch June 19, 2020 02:31

rushtehrani changed the title ~~feat: runtime variables and system config change reload~~ feat: Runtime variables and system config change reload Jun 20, 2020

feat: Runtime variables and system config change reload #352

feat: Runtime variables and system config change reload #352

Uh oh!

Conversation

Vafilor commented Jun 16, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rushtehrani commented Jun 17, 2020

Uh oh!

Vafilor commented Jun 17, 2020

Uh oh!

rushtehrani commented Jun 17, 2020

Uh oh!

rushtehrani commented Jun 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vafilor commented Jun 17, 2020

Uh oh!

rushtehrani commented Jun 17, 2020

Uh oh!

Vafilor commented Jun 17, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rushtehrani commented Jun 17, 2020

Uh oh!

Vafilor commented Jun 17, 2020

Uh oh!

rushtehrani commented Jun 18, 2020

Uh oh!

rushtehrani commented Jun 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rushtehrani commented Jun 17, 2020 •

edited

Loading