Skip to content
This repository was archived by the owner on Jan 21, 2022. It is now read-only.

kubernetes container manager#41

Open
MHBauer wants to merge 1 commit intocloudfoundry-attic:masterfrom
MHBauer:kube-cm
Open

kubernetes container manager#41
MHBauer wants to merge 1 commit intocloudfoundry-attic:masterfrom
MHBauer:kube-cm

Conversation

@MHBauer
Copy link
Copy Markdown
Collaborator

@MHBauer MHBauer commented Sep 13, 2018

Resolves #9
Resolves #66
Resolves #67

PoC on start of containermanager to get the deps right. For #9

  • document your feature
  • comment your code
  • update the README / how-to-run
  • have tests
  • get reviews

not intended to merge yet, want to hold the deps so it's shown.

@cfdreddbot
Copy link
Copy Markdown

Hey MHBauer!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@MHBauer
Copy link
Copy Markdown
Collaborator Author

MHBauer commented Sep 13, 2018

rebased on #40 and split into two commits

I suggest looking at gopkg.toml for the dep changes, and the second commit for the actual code changes so far.

@MHBauer MHBauer changed the title WIP - a kubernetes based container manager that compiles and runs WIP - a kubernetes based container manager that compiles and runs for #9 Sep 13, 2018
Comment thread cmd/broker/main.go Outdated
var manager containermanager.ContainerManager

manager := docker.NewDockerContainerManager(logger, cli)
if cli, err := client.NewEnvClient(); err != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make it configurable in the config file to choose the underlying engine to run the broker.

We can have smoething like "engine": "docker" or "engine": "kubernetes" in the config file and decide on what to use based on what the property is set to. defaulting to docker.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done in #52!

@MHBauer
Copy link
Copy Markdown
Collaborator Author

MHBauer commented Sep 28, 2018

this is now super tiny with #52 in. yay.

@MHBauer MHBauer changed the title WIP - a kubernetes based container manager that compiles and runs for #9 kubernetes container manager Oct 4, 2018
@MHBauer
Copy link
Copy Markdown
Collaborator Author

MHBauer commented Oct 4, 2018

works e2e as demonstrated today. needs rebase.

Comment thread pkg/deployer/deployer.go
Args []string `json:"args"`
}{
Provider: fmt.Sprintf("http://%s:%s", containerInfo.InternalAddress, portBindings[0].Port),
Provider: fmt.Sprintf("http://%s:%s", containerInfo.InternalAddress, "8545"),
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think this is an important distinction and maybe our struct needs to have internalPortmap and externalPortmap

@@ -1,4 +1,4 @@
# client-go
re# client-go
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

oops

Comment thread pkg/deployer/deployer.go
if err != nil {
return nil, err
}
e.logger.Debug(fmt.Sprintf("%++v", config))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is helpful to see that the config was wrong

Comment thread pkg/deployer/deployer.go
defer os.RemoveAll(outputFile.Name())

cmd := exec.Command("node", e.deployerPath, "-c", configFile.Name(), "-o", outputFile.Name(), contractInfo.ContractPath)
e.logger.Debug(fmt.Sprintf("%++v", cmd))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is heplful to see what the cmd looks like before it gets run. make sure all the args are present.

func NewKubernetesContainerManager(logger lager.Logger, client kubernetes.Interface, host string) containermanager.ContainerManager {
return kubernetesContainerManager{
client: client,
host: host,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

external address from config. maybe needs to be renamed

Comment thread k8s/service-cm.yaml
@@ -0,0 +1,26 @@
kind: ConfigMap
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

unused for now

Comment thread k8s/rbac.yaml
subjects:
- kind: ServiceAccount
name: default
namespace: default
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

should be set to be more restrictive.
create pods and svc
delete pods and svc

Comment thread k8s/n.yaml
@@ -0,0 +1,45 @@
---
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

for screwing around with ingress

Comment thread k8s/ingress.yaml
@@ -0,0 +1,16 @@
apiVersion: extensions/v1beta1
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

useful if you want to have the broker have a clean URL.

Comment thread k8s/deploy.yaml
spec:
ports:
- port: 3333
nodePort: 30000
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

hardcoded for ease of repros for now.

Comment thread k8s/deploy.yaml
"port": 3333,
"deployer_path":"/pusher.js",
"container_manager": "kubernetes",
"external_address": "peanuts.sng01.containers.appdomain.cloud"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

edit this line with your load-balancer DNS or ip of worker node.

@MHBauer
Copy link
Copy Markdown
Collaborator Author

MHBauer commented Nov 7, 2018

Comment thread k8s/service-cm.yaml
@@ -0,0 +1,26 @@
kind: ConfigMap
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this can be considered incomplete work. If we want to avoid hardcoding the config directly into the image, this needs to be loaded by changing the configuration in k8s/deploy.yaml

We can save this file into an issue and put it off to be done later.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the biggest thing is that it would avoid changes to services/eth.json while also risking it becoming out of sync.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sounds good. will create a chore to track this

@MHBauer
Copy link
Copy Markdown
Collaborator Author

MHBauer commented Nov 27, 2018

linked appropriately

comments on how to ingress

nodeport service config

bind

remove ingress

static build so I don't have to worry about running things

also it's smaller

might as well be a nodeport

comments on what things are

use of config

configmap for config info for k8s backend

configmap can't be mounted at root, as it will overwrite whole FS

mount under subdir and adjust command to load config from that directory

pusher config and correct slice

temporarily change return values

maybe mount service definitions as well

fixup rebase changes

start of instructions

more instructions

more instructions and try and make it work again

internal binding for internal address

missing test file

specific rbac

prepend an arbitrary character before service use of instance id

B, for Blockhead!
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants