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 packageLookup package and osDiscovery package #9

Merged
merged 11 commits into from
Jun 15, 2017

Conversation

lobiCode
Copy link
Contributor

-tags flag is not needed any more when we build a code,
because package osDiscovery provide functionality to get all
the basic system information data. According to distribution provided
by osDiscovery, installed packages are fetched with package
packageLookup.

@lobiCode lobiCode force-pushed the add-sub-pacakges branch 2 times, most recently from 4763c6e to e32915f Compare May 25, 2017 13:01
Copy link
Contributor

@dominikznidar dominikznidar left a comment

Choose a reason for hiding this comment

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

First batch of comments

.travis.yml Outdated
- VERSION=${TRAVIS_TAG:=$(cat ./VERSION)} make pkg/$DISTRIBUTION
- sed -i'' -e "s/\DISTRIBUTION/${DISTRIBUTION}/" descriptor.json
- VERSION=${TRAVIS_TAG:=$(cat ./VERSION)} make pkg/$PACKAGE_MANAGER
- sed -i'' -e "s/\PACKAGE_MANAGER/${PACKAGE_MANAGER}/" descriptor.json
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Set version env to current version and build packages.
  2. replace PACKAGE_MANAGER with package manager from matrix in descriptor.json file.
    I forgot to change DISTRIBUTION in descriptor.json file with PACKAGE_MANAGER or maybe I should ask before I changed those lines because I don't know what consequence this will have for deploying process.

Makefile Outdated
@@ -8,7 +8,8 @@ CI_COMMIT ?= dev
GIT_COMMIT := $(shell git rev-parse --short HEAD || echo $(CI_COMMIT))
VERSION ?= $(shell cat ./VERSION)
FLAGS := "-X main.GitCommit=$(GIT_COMMIT) -X main.Version=$(VERSION)"
DISTRIBUTIONS := ubuntu debian rhel centos opensuse sles
DISTRIBUTIONS := ubuntu debian rhel centos opensuse sles amzn
PACKAGE_MANAGER := dpkg rpm
Copy link
Contributor

Choose a reason for hiding this comment

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

all lines above this one are nicely aligned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Makefile Outdated
@echo "*\n* Testing for $* locally \n*"
go test $(TEST_FLAGS) -tags $*
test: vet
go test -v
Copy link
Contributor

Choose a reason for hiding this comment

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

you could also just run go test ./...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. change to go test ./...

Makefile Outdated

format: ## formats the code
gofmt -w $(SOURCE)

vet: ## examines the go code with `go vet`
go vet $(SOURCE)
go vet
Copy link
Contributor

Choose a reason for hiding this comment

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

this does not find vet issues in subfolders, go vet ./... does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. change to go vet ./...

Makefile Outdated

format: ## formats the code
gofmt -w $(SOURCE)

vet: ## examines the go code with `go vet`
go vet $(SOURCE)
go vet

up: $(addprefix up/,$(DISTRIBUTIONS)) ## start agents for all distributions
up/%: ## starts the agent for a specific distribution
docker-compose --project-name=tactycal up agent$*

run/%:
Copy link
Contributor

Choose a reason for hiding this comment

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

you made /% obsolete

Copy link
Contributor Author

@lobiCode lobiCode May 30, 2017

Choose a reason for hiding this comment

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

fixed. /% has been removed. docker-compose.ym and docker-compose.tmpl files have been also updated.

.travis.yml Outdated
- DISTRIBUTION=rhel
- DISTRIBUTION=opensuse
- DISTRIBUTION=sles
- PACKAGE_MANAGER=dpkg
Copy link
Contributor

Choose a reason for hiding this comment

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

can you change this from dpkg / rpm to deb / rpm as those will be the names of repositories in bintray.com. You can then also replace PACKAGE_MANAGER with PACKAGE_TYPE.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

  • PACKAGE_MANAGER has been replaced with PACKAGE_TYPE
  • dpkg has been replaced with deb

Makefile Outdated
test/%: vet ## runs unit tests for a specific distribution
@echo "*\n* Testing for $* locally \n*"
go test $(TEST_FLAGS) -tags $*
test: vet
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment that adds the method to the help screen (make help) is missing. Can you add it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Comment has been added.

Makefile Outdated

format: ## formats the code
gofmt -w $(SOURCE)

vet: ## examines the go code with `go vet`
go vet $(SOURCE)
go vet ./...

up: $(addprefix up/,$(DISTRIBUTIONS)) ## start agents for all distributions
up/%: ## starts the agent for a specific distribution
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also do a slight change here? If you change up/%: to up/%: build we can also stop building custom images for all the repos, just mount build folder to them in docker-compose and execute the binary in them.
In short, can you change this so that we run the generated binary inside docker containers and not run the source code with go installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

if release, err := execCommand("lsb_release", "-r"); err == nil {
return string(release)
}
func GetHostInfo() (*Host, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a test for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

@@ -0,0 +1,234 @@
package packageLookup
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put this in a separate folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stubUtils package has been added.

@lobiCode lobiCode force-pushed the add-sub-pacakges branch 2 times, most recently from db83c88 to 75fb4b7 Compare June 1, 2017 10:58
@@ -4,7 +4,8 @@ services:
build:
context: ./
dockerfile: Dockerfile.debian
working_dir: /go/src/agent
working_dir: /go/src/github.com/tactycal/agent
command: ./build/usr/bin/tactycal -f my_conf.conf -s /state/agnet_state -t 3s -d
Copy link
Contributor

Choose a reason for hiding this comment

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

agent -> agent

Copy link
Contributor

Choose a reason for hiding this comment

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

agnet

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed in my commit

@dominikznidar dominikznidar force-pushed the add-sub-pacakges branch 2 times, most recently from 3b87872 to 13d50c7 Compare June 15, 2017 11:47
Uros Slana and others added 11 commits June 15, 2017 13:56
`-tags` flag is not needed any more when we build a code,
because package **osDiscovery** provide functionality to get all
the basic system information data. According to distribution provided
by **osDiscovery**, installed packages are fetched with package
**packageLookup**.
Also removes the docker-compose template as we were not saving any space with it.
@dominikznidar dominikznidar merged commit 8840218 into master Jun 15, 2017
@dominikznidar dominikznidar deleted the add-sub-pacakges branch June 15, 2017 12:09
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.

None yet

2 participants