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.

Sign up for GitHub

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

Jump to bottom

Migration to the new registry (Harbor) #1883

Merged
merged 10 commits into from
May 17, 2022
Merged

Migration to the new registry (Harbor) #1883

merged 10 commits into from
May 17, 2022

Conversation

gruberdev
Copy link
Contributor

@gruberdev gruberdev commented May 6, 2022 •
edited by github-actions bot
Loading

React Kit Coverage

Breaking Changes

No. Only adds the new registry to the repository.

Runway Ticket URL

GOG-594

thestephenmarshall and others added 4 commits May 5, 2022 10:18
@thestephenmarshall @ericawinne
Converting title kit to ts (#1872)
48404b6 
* Removed flow and Renamed file

* Updated Types

* Removed JSX file extentions from imports

* Fix syntax error

* Fix tests

Co-authored-by: Erica Winne <[email protected]>
@thestephenmarshall @ericawinne
Converting caption to tsx (#1874)
9a31b00 
* Started Converting

* Rename file to TSX and Updated types

* Added/Joined & GlobalProps

* Merge Master

* Changed back default blank string

Co-authored-by: Erica Winne <[email protected]>
@KurstynStorms @thestephenmarshall
[PLAY-142] Visual Guidelines - Switch to React - Examples Template (#…
8abdad4 
…1849)

* Rendered rough example in browser

* Section separator between props and values

* Added pills and continued to change styling

* Fixed width of cards

* Fixed flexitem alignment

* Added title and body kits

* Fixed more styling

* Changed out background kit to fix border radius

* Fixed border radius

* Fixed border radius on other templates

* Updated caption card

* Fixed title margins

* Corrected camelcase in pills

* Three templates for visual guidelines

* Fixed syntax error

* Fixed syntax again

* Testing client-side code example colorization

* Use Rouge to format code examples

* Template Complete

Co-authored-by: Stephen Marshall <[email protected]>
@gruberdev
[feat/new_registry] Added Harbor URI to replace Quay.io
7aa4e54
@gruberdev gruberdev added enhancement New Features, Props, & Variants (USED IN CHANGELOG) milano 20 MAX - Deploy this PR to a review environment via Milano labels May 6, 2022
@gruberdev gruberdev requested a review from a team as a code owner May 6, 2022 19:09
@gruberdev gruberdev changed the title Added new container registry (Harbor) Migration to new registry (Harbor) May 6, 2022
@thestephenmarshall
Copy link
Contributor

@gruberdev this makes sense to me, but could you get this to deploy please?

jasperfurniss
jasperfurniss reviewed May 6, 2022
View reviewed changes
Jenkinsfile
@@ -19,7 +19,7 @@ app.build(

stage('Code Checkout') {
scmVars = checkout scm
appImage = "quay.io/powerhome/playbook:${git.triggeringCommit(scmVars)}"
appImage = "image-registry.powerapp.cloud/playbook/playbook:${git.triggeringCommit(scmVars)}"
Copy link
Contributor

@jasperfurniss jasperfurniss May 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Would we need to tweak 3 more files i believe in order for this to work? @gruberdev
Please look at the following files and see if they are relevant for this Pull Request.
Also, we definitely need a full description on this work. We don't know the intention behind this pull request.

  1. playbook/playbook-website/bin/deployer

    Line 10 in 284aea6

    DEPLOYER_IMAGE="quay.io/powerhome/deployer:master-5246eafb24c81acae0dd9ccc3e421d0bb8d18328-1288"

  2. playbook/playbook-website/config/deploy/templates/deployment.yaml.erb

    Line 23 in be09fa5

    image: "quay.io/powerhome/playbook:<%= image_tag %>"

  3. playbook/Jenkinsfile

    Line 22 in be09fa5

    appImage = "quay.io/powerhome/playbook:${git.triggeringCommit(scmVars)}"

  4. playbook/Makefile

    Line 8 in be09fa5

    DEPLOYER_IMAGE = quay.io/powerhome/deployer:master-c94bf553840b07335fbb8904d5a9963dd5ffce00-336

thestephenmarshall
thestephenmarshall requested changes May 6, 2022
View reviewed changes
Copy link
Contributor

@thestephenmarshall thestephenmarshall left a comment

Choose a reason for hiding this comment

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

Not sure what the scope on this PR is. Was it the intent to change multiple files? Is this PR still WIP? I think we just need a little more clarification as to what is happening here.

@gruberdev gruberdev added milano 20 MAX - Deploy this PR to a review environment via Milano and removed milano 20 MAX - Deploy this PR to a review environment via Milano labels May 6, 2022
@gruberdev
Copy link
Contributor Author

gruberdev commented May 6, 2022 •
edited
Loading

@gruberdev this makes sense to me, but could you get this to deploy please?

Indeed, Milano uses a missing reference in a template to deploy images from the registry; I believe it should work after 6c8fa6b.

Would we need to tweak 3 more files i believe in order for this to work? @gruberdev Please look at the following files and see if they are relevant for this Pull Request. Also, we definitely need a full description on this work. We don't know the intention behind this pull request.

  1. playbook/playbook-website/bin/deployer

    Line 10 in 284aea6

    DEPLOYER_IMAGE="quay.io/powerhome/deployer:master-5246eafb24c81acae0dd9ccc3e421d0bb8d18328-1288"

  2. playbook/playbook-website/config/deploy/templates/deployment.yaml.erb

    Line 23 in be09fa5

    image: "quay.io/powerhome/playbook:<%= image_tag %>"

  3. playbook/Jenkinsfile

    Line 22 in be09fa5

    appImage = "quay.io/powerhome/playbook:${git.triggeringCommit(scmVars)}"

  4. playbook/Makefile

    Line 8 in be09fa5

    DEPLOYER_IMAGE = quay.io/powerhome/deployer:master-c94bf553840b07335fbb8904d5a9963dd5ffce00-336

The reference to the master image used seems to be of a specific version, as to where I don't have enough context to attest to whether it is adequate to use the latest master build to replace it. However, I've added the image generated from last build in its place; let me know if this change works.

Not sure what the scope on this PR is. Was it the intent to change multiple files? Is this PR still WIP? I think we just need a little more clarification as to what is happening here.

This PR aims to migrate the image building and storage from Quay.io to our newer internal registry that uses Harbor as a container registry platform.

The PR's original purpose worked as intended since the image built was successfully uploaded to the newer internal container registry. It is worth noting I was not aware of the additional changes that would have to be made, so it can be considered a WIP, although using the feedback provided until now, I made a few further changes that make the PR's proposal more robust.

Any feedback regarding these changes is welcome as well.

@gruberdev gruberdev added the wip label May 6, 2022
@gruberdev gruberdev removed the wip label May 6, 2022
@app-milano app-milano bot temporarily deployed to pr1883 May 6, 2022 23:18 Inactive
@benlangfeld benlangfeld requested a review from a team May 9, 2022 13:45
@gruberdev gruberdev changed the title Migration to new registry (Harbor) Migration to the new registry (Harbor) May 9, 2022
benlangfeld
benlangfeld reviewed May 10, 2022
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@app-milano app-milano bot temporarily deployed to pr1883 May 10, 2022 22:39 Inactive
@thestephenmarshall thestephenmarshall removed the enhancement New Features, Props, & Variants (USED IN CHANGELOG) label May 11, 2022
@gruberdev gruberdev merged commit ab9d75e into master May 17, 2022
@gruberdev gruberdev deleted the feat/new_registry branch May 17, 2022 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasperfurniss jasperfurniss jasperfurniss left review comments

@thestephenmarshall thestephenmarshall thestephenmarshall approved these changes

@benlangfeld benlangfeld benlangfeld approved these changes

Assignees
No one assigned
Labels
milano 20 MAX - Deploy this PR to a review environment via Milano
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gruberdev @thestephenmarshall @benlangfeld @jasperfurniss @KurstynStorms