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

Add grpc api #48

Merged
merged 10 commits into from
Jul 9, 2021
Merged

Add grpc api #48

merged 10 commits into from
Jul 9, 2021

Conversation

jmleefree
Copy link
Member

  • ladybug grpc-api 폴더 추가
    • cbadm/cmd : grpc cli 툴
    • cbadm/test : docs/test 기본으로 해서 grpc 에서 동작하도록 구성
    • cert-gen : grpc tls 통신할 때 필요한 인증서 생성
    • common : grpc common 함수 모음
    • config : 환경파일 로딩 함수
    • docker : grpc 관련 backend 서버 제공
    • interceptors : grpc 인터셉터
    • jwt-gen : jwt 토큰 생성
    • logger : 로거 함수
    • protobuf : protobuf 파일 저장
    • request : grpc client 및 go api 제공
    • server : grpc server 제공
  • conf 폴더에 grpc 환경설정 파일 grpc_conf.yaml 추가
  • main.go 에 grpc 서버도 실행하도록 추가함
  • .gitignore 에 cbadm 바이너리 제외 추가

@sykim-etri
Copy link
Member

@jmleefree 카페모카 버전 릴리즈 후 리뷰 및 머지할 예정이니 일정에 참고하시기 바랍니다.

@jihoon-seo
Copy link
Member

go.mod 에 conflict 가 발생하여, resolve 했습니다. (8e205c6)

그런데, build 에러가 나네요

https://github.com/cloud-barista/cb-ladybug/pull/48/checks?check_run_id=2970529882#step:3:71

Step 5/19 : RUN GOOS=linux CGO_ENABLED=0 go build -ldflags '-w -extldflags "-static"' -tags cb-ladybug -o cb-ladybug -v src/main.go
 ---> Running in 310d4ecaf6c8
go: github.com/cloud-barista/cb-log@v0.4.0: missing go.sum entry; to add it:
	go mod download github.com/cloud-barista/cb-log
The command '/bin/sh -c GOOS=linux CGO_ENABLED=0 go build -ldflags '-w -extldflags "-static"' -tags cb-ladybug -o cb-ladybug -v src/main.go' returned a non-zero code: 1

Error: Process completed with exit code 1.

제가 conflict 를 resolve 할 때 go.sum 은 수정하지 않아서 발생하는 에러입니다.

제가 좀 더 탐구해 보겠습니다.

@jihoon-seo
Copy link
Member

/lgtm
필요하다면 squash 를 하는 방안도 있겠습니다.

@github-actions github-actions bot added the lgtm label Jul 2, 2021

// ===== [ Public Functions ] =====

// SetupAndRun - SPIDER GRPC CLI 구동
Copy link
Member

Choose a reason for hiding this comment

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

SPIDER 부분은 수정되진 못한 부분으로 보이는데, 검토 부탁드립니다.

return clusterCmd
}

// NewClusterCreateCmd - Cloud Cluster 생성 기능을 수행하는 Cobra Command 생성
Copy link
Member

Choose a reason for hiding this comment

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

용어의 일관성을 위해
주석 부분에 있는 'Cloud Cluster' -> 'Cluster', 'Cloud Node' -> 'Node'로 수정과
cobra.Command{Short: xxx, Long: xxx}에 'cloud cluster' -> 'cluster', 'cloud node' -> 'node'로 수정 부탁드립니다.


prometheus-lb:
image: prom/prometheus
container_name: etri_prometheus_lb
Copy link
Member

Choose a reason for hiding this comment

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

container_name에 etri_XXX 등으로 포함된 경우가 몇개 있습니다. etri보다는 cloud-barista나 cb로 수정을 부탁드립니다.

var (
jwtKey = flag.String("jwt-key", "", "The JWT Signing Key")
userName = flag.String("user", "HongGilDong", "The User Name")
orgName = flag.String("org", "ETRI", "The Organization Name")
Copy link
Member

Choose a reason for hiding this comment

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

ETRI 대신 Cloud-Barista가 적당해 보입니다.

}

//////////////////////////////////
// ClUSTER 메시지 정의
Copy link
Member

Choose a reason for hiding this comment

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

ClUSTER 단순 오타(l) 수정

@@ -0,0 +1,87 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

src/grpc-api/cdadm/test/.sh가 docs/test/.sh와 거의 유사한 것으로 보입니다. 혹시 통합하여 활용할 방법은 없을지요?

@sykim-etri
Copy link
Member

@itnpeople @vlatte docs/test의 *.sh들이 GRPC 테스트 지원을 위해 CB_CALL_METHOD라는 환경 변수를 기준으로 일부 변경되었습니다. REST API 테스트 코드는 그대로 들어가 있습니다만 테스트가 되진 않았을 것으로 판단됩니다. 리뷰하시고 의견 부탁드립니다.

@@ -47,6 +47,8 @@ echo "- Cluster name is '${v_CLUSTER_NAME}'"
# Create a cluster
create() {

source ./conf.env
Copy link
Member

Choose a reason for hiding this comment

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

혹시 해당 .sh의 상단에서 source ./conf.env를 수행하면 문제가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

상관없습니다. 상단으로 올릴까요?

Copy link
Member

Choose a reason for hiding this comment

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

@jmleefree 각 스크립트 상단에 참조하는 환경 설정 파일을 기재하는 것이 처음 접하는 사람들에게는 혼란을 덜 줄 것으로 기대됩니다.
usage 출력하는 부분 아래에 추가하면 적당할 것 같습니다.

@vlatte
Copy link
Collaborator

vlatte commented Jul 9, 2021

@sykim-etri test script 중 REST API 부분 테스트 완료하였습니다.

@sykim-etri
Copy link
Member

@vlatte 테스트까지 해주셨군요. 코드상으로만 특이한 점이 없는지만 봐주셨어도 되는데.. 감사합니다.

@sykim-etri sykim-etri merged commit e65ec47 into cloud-barista:master Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants