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

Fix Docker Hub CI test builds and update Makefile #756

Merged
merged 1 commit into from Jul 29, 2020
Merged

Fix Docker Hub CI test builds and update Makefile #756

merged 1 commit into from Jul 29, 2020

Conversation

sakalosj
Copy link
Contributor

@sakalosj sakalosj commented Jul 28, 2020

Because of Docker Hub CI test builds are triggered for all branches we need
set SOURCE_BRANCH to master in case a custom branch is used.

Makefile was updated because image builds require SOURCE_BRANCH
build variable.

@sakalosj sakalosj changed the title Fix Docker Hub CI test builds WIP: Fix Docker Hub CI test builds Jul 28, 2020
@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build failed.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Because of Docker Hub CI test builds are triggered for all branches we need
set SOURCE_BRANCH to master in case a custom branch is used.

Makefile was updated because image builds require SOURCE_BRANCH
build variable.
@sakalosj sakalosj changed the title WIP: Fix Docker Hub CI test builds Fix Docker Hub CI test builds and update Makefile Jul 28, 2020
@sakalosj sakalosj added the ready-for-review Pull request is ready for review. label Jul 28, 2020
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

fail:
msg: Variable source_branch, which is set from env variable SOURCE_BRANCH is empty.
when: source_branch == ''
# Docker Hub CI test is build from non master/stable branch (eg. packit:fix_hook)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Docker Hub CI test is build from non master/stable branch (eg. packit:fix_hook)
# Docker Hub CI image builds are performed from non contributor's branches (eg. packit:fix_hook), but we need it to be master or stable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

forgot to include it now, but will do it another commit

@csomh
Copy link
Contributor

csomh commented Jul 29, 2020

Sorry, but I don't remember, how is SOURCE_BRANCH used? Why is it needed and how does it's value influence the build?

@sakalosj
Copy link
Contributor Author

Sorry, but I don't remember, how is SOURCE_BRANCH used? Why is it needed and how does it's value influence the build?

SOURCE_BRANCH contains a current branch. Based on it, a master/stable branch from other projects will be chosen. introduced in #717


worker: files/install-deps-worker.yaml files/recipe-worker.yaml
$(CONTAINER_ENGINE) build --rm -t $(WORKER_IMAGE) -f files/docker/Dockerfile.worker .
$(CONTAINER_ENGINE) build --rm -t $(WORKER_IMAGE) -f files/docker/Dockerfile.worker --build-arg SOURCE_BRANCH=$SOURCE_BRANCH .

# This is for cases when you want to deploy into production and don't want to wait for dockerhub
# Make sure you have latest docker.io/usercont/packit:prod prior to running this
worker-prod: files/install-deps-worker.yaml files/recipe-worker.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this target be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will not remove it in this PR because it is used in different places and probably will require some additional adjustments

raise an issue for this #759

@csomh
Copy link
Contributor

csomh commented Jul 29, 2020

Sorry, but I don't remember, how is SOURCE_BRANCH used? Why is it needed and how does it's value influence the build?

SOURCE_BRANCH contains a current branch. Based on it, a master/stable branch from other projects will be chosen. introduced in #717

A case insensitive grep would have helped me with finding this answer, but yeah, next time :)
Thanks!

@sakalosj
Copy link
Contributor Author

Sorry, but I don't remember, how is SOURCE_BRANCH used? Why is it needed and how does it's value influence the build?

SOURCE_BRANCH contains a current branch. Based on it, a master/stable branch from other projects will be chosen. introduced in #717

A case insensitive grep would have helped me with finding this answer, but yeah, next time :)
Thanks!

np ;)

@sakalosj
Copy link
Contributor Author

@TomasTomecek @csomh

thanks for the review, but sorry I forgot that I was "testing" this and it requires to be moved to separate file and processed by both install-deps ansible playbooks

will implement re-request review

@sakalosj
Copy link
Contributor Author

@TomasTomecek @csomh

thanks for the review, but sorry I forgot that I was "testing" this and it requires to be moved to separate file and processed by both install-deps ansible playbooks

will implement re-request review

pls ignore - will implement it in other PR which is focused on install-deps.yaml specifically #757

@sakalosj sakalosj added the mergeit When set, zuul wil gate and merge the PR. label Jul 29, 2020
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mergeit When set, zuul wil gate and merge the PR. ready-for-review Pull request is ready for review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants