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

feat: builder docker/node and sdk-js #1158

Merged
merged 14 commits into from
Nov 13, 2020
Merged

feat: builder docker/node and sdk-js #1158

merged 14 commits into from
Nov 13, 2020

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Oct 9, 2020

This PR is aligned with the efforts in testground/sdk-js#1.

  • Adds a docker/node builder with the following base Dockerfile:
     ARG BASE_IMAGE
     FROM ${BASE_IMAGE} AS builder
     ENV PLAN_DIR /plan
     WORKDIR /plan
     COPY . /
     RUN npm ci
     EXPOSE 6060
     ENTRYPOINT [ "npm", "start"]
    
  • Adds a test plan example-js with the following tests:
    • pingpong
    • sync
    • failure
    • output (not complete because metrics aren't finished yet)
  • Adds example-js/pingpong to CI

@hacdias
Copy link
Member Author

hacdias commented Oct 10, 2020

We have to make network.Config the most compatible possible. Next week, I will tackle on what @raulk mentioned in the linked PR and will also implement an Unmarshaller/Decoder for that struct. Done: testground/sdk-go#34

Some things such as net.IPNet are not easily converted to the same thing as Go. My suggestion is to just pass around the string CIDR. That will virtually work on all implementations since there's always a function to convert CIDRs to native structs.

We can keep using net.IPNet in Go. I'll just need to implement an unmarshaller.

@raulk
Copy link
Contributor

raulk commented Oct 10, 2020

Yeah, I think struct tags will do for the casing. And for IPNet, there's actually a marshaller/unmarshaller you can use. Let me try digging it up!

@raulk
Copy link
Contributor

raulk commented Oct 10, 2020

Here it is: https://github.com/testground/sdk-go/blob/master/runtime/runparams.go

@hacdias
Copy link
Member Author

hacdias commented Oct 10, 2020

Just throwing it out again: I think we should collect all this types in some place! 😆 (cycles!) Gonna move IPNet to ptypes!

@hacdias
Copy link
Member Author

hacdias commented Oct 16, 2020

I'm changing the base of this PR to #1154 because that one includes the newer SDK with the updates to all tests. However, I'd prefer if we could merge #1154 into master when possible, since that one is not even based on master.

@hacdias
Copy link
Member Author

hacdias commented Oct 16, 2020

Update: ping-pong test working!

Base automatically changed from tasks-status to add-stdout-to-logs October 16, 2020 18:31
Base automatically changed from add-stdout-to-logs to master October 16, 2020 18:34
@hacdias hacdias mentioned this pull request Oct 17, 2020
4 tasks
@hacdias hacdias changed the title feat: builder docker/go and sdk-js feat: builder docker/node and sdk-js Nov 6, 2020
License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
License: MIT
Signed-off-by: Henrique Dias <hacdias@gmail.com>
@hacdias
Copy link
Member Author

hacdias commented Nov 6, 2020

@nonsense @raulk I feel this PR is finished. Even though sdk-js isn't complete (missing metrics before releasing 1.0), I feel this PR is ready for review and should be merged. Later, we can make further changes if needed.

@hacdias hacdias marked this pull request as ready for review November 6, 2020 10:46
@hacdias hacdias requested review from raulk and nonsense and removed request for raulk November 6, 2020 10:46
pkg/build/docker_node.go Outdated Show resolved Hide resolved
pkg/build/docker_node.go Outdated Show resolved Hide resolved
Copy link
Member

@nonsense nonsense left a comment

Choose a reason for hiding this comment

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

Overall LGTM!

Co-authored-by: Anton Evangelatov <anton.evangelatov@gmail.com>
@hacdias
Copy link
Member Author

hacdias commented Nov 13, 2020

I'm going to address the comments and then merge!

@hacdias hacdias merged commit 87bd839 into master Nov 13, 2020
@hacdias hacdias deleted the docker-js branch November 13, 2020 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants