Skip to content
This repository has been archived by the owner on Dec 11, 2023. It is now read-only.

implement ci #57

Merged
merged 6 commits into from Nov 24, 2020
Merged

implement ci #57

merged 6 commits into from Nov 24, 2020

Conversation

sameersbn
Copy link
Contributor

@sameersbn sameersbn commented Nov 20, 2020

  • added Makefile
  • added circle ci config
  • updated Dockerfiles for implement ci #57 changes
  • sync runtime dir names with target image names
  • Dockerfiles updated to existing source repo instead of downloading a release of KLR

closes #52

Makefile Outdated
IMAGES = $(foreach r,$(RUNTIMES),$(r).image)
images: $(IMAGES) ## Build the Docker images
$(IMAGES): %.image:
@docker build -t $(IMAGE_REPO)/knative-lambda-$$(echo "$*" | sed -n -e "s/\([[:alnum:]]*\)\(-\)*\([0-9]*\)\(\.\)*\([0-9]*\)\(\.\)*\([0-9]*\).*/\1\3\5\7/p") $*

Choose a reason for hiding this comment

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

😨

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea. it's a bit of a stretch.
We could get rid of the voodoo stuff if we just renamed the folders to their image names, f.e. python-2.7 -> python27

Copy link
Member

Choose a reason for hiding this comment

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

renaming folders would require updates in several documents that refer to KLR runtime yaml files as well as in CLI bits that are also using KLR yamls. Maybe we could just list folders by their current names to avoid renaming?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we could just list folders

do you mean link?

Copy link
Member

Choose a reason for hiding this comment

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

nope, I need to think about what could be the solution, please don't merge yet. Although, maybe having the unified folder name structure actually worth updating all related docs and CLI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tzununbekov i'll revert my folder renaming change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tzununbekov i've reverted the folder rename change. please take a look

Copy link
Member

Choose a reason for hiding this comment

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

well, the only possible way other than moving current paths is walking through the existing folders and grep-ing runtime.yaml for FROM gcr.io/triggermesh string to see which image is being used by the runtime. This also feels really hackish, so if we decide to go with the renaming approach, I will not argue - we'll just need to update a couple of files in the tm CLI and a bunch of them in the current repository.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

feels really hackish

That's true.

need to update a couple of files in the tm CLI and a bunch of them in the current repository.

I can work on this change. Are there any docs that would need updating?

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be easier for me to update those references. Give me 20 minutes, I'll link related PRs

@sameersbn
Copy link
Contributor Author

@tzununbekov please take a look. updated the PR description to give an overview of the changes

@sameersbn sameersbn merged commit bda5601 into master Nov 24, 2020
@sameersbn sameersbn deleted the add-ci branch November 24, 2020 12:10
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.

add CI infrastructure
3 participants