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 docs for Docker Hub repo support in Advanced Agent set up #760

Closed
wants to merge 5 commits into from

Conversation

KyleGoyette
Copy link
Contributor

@KyleGoyette KyleGoyette commented Jul 17, 2024

Description

What does the pull request do? If it fixes a bug or resolves a feature request, be sure to link to that issue.

Screenshot for πŸ‘οΈ
image

Ticket

Does this PR fix an existing issue? If yes, provide a link to the ticket here:

Checklist

Check if your PR fulfills the following requirements. Put an X in the boxes that apply.

  • Files I edited were previewed on my local development server with yarn start. My changes did not break the local preview.
  • Build (yarn docusaurus build) was run locally and successfully without errors or warnings.
  • I merged the latest changes from main into my feature branch before submitting this PR.

@KyleGoyette KyleGoyette requested a review from a team as a code owner July 17, 2024 22:36
@ngrayluna
Copy link
Contributor

@KyleGoyette Is this PR ready for review, merge?

destination: <organization|user>/<repo-name>
# If using Kaniko, specify the PVC or cloud bucket where the agent can store the
# build context.
build-context-store: <PVC | cloud bucket>
Copy link
Contributor

Choose a reason for hiding this comment

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

You're not supposed to specify a pvc here, you just do it via the helm chart. We need to know the pvc name and where it is mounted in the agent pod and that gets passed through environment variables set in the chart

Copy link
Contributor Author

@KyleGoyette KyleGoyette Jul 29, 2024

Choose a reason for hiding this comment

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

Ah okay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So.... if a PVC is specified in the helm chart leave this blank?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this should be left blank. This relates to the coupling of the agent config and helm chart. Users control the literal contents of the agent config as a chart variable and provide a pvc name as another chart variable. Because they control the literal contents I think it would be weird for us to inject the pvc name into the agent config. Or ask them to write it in both places.

Copy link
Contributor

Choose a reason for hiding this comment

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

In retrospect it would be better to have helm variables for the agent config fields and then construct that config in the agent pod for the user.

Copy link

cloudflare-workers-and-pages bot commented Oct 3, 2024

Deploying docodile with Β Cloudflare Pages Β Cloudflare Pages

Latest commit: 22bd0c6
Status:Β βœ…Β  Deploy successful!
Preview URL: https://09dd5cbc.docodile.pages.dev
Branch Preview URL: https://launch-dockerhub-repo.docodile.pages.dev

View logs

@@ -23,7 +23,7 @@ The Launch agent can build images using [Docker](https://docs.docker.com/) or [K
* Kaniko: builds a container image in Kubernetes without running the build as a privileged container.
* Docker: builds a container image by executing a `docker build` command locally.

The builder type can be controlled by the `builder.type` key in the launch agent config to either `docker`, `kaniko`, or `noop` to disable build. By default, the agent helm chart sets the `builder.type` to `noop`. Additional keys in the `builder` section will be used to configure the build process.
The builder type can be controlled by the `builder.type` key in the launch agent config to either `docker`, `kaniko`, or `noop` to disable build. By default, the agent Helm chart sets the `builder.type` to `noop`. Additional keys in the `builder` section will be used to configure the build process.
Copy link

Choose a reason for hiding this comment

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

πŸ“ [vale] reported by reviewdog 🐢
[Google.Passive] In general, use active voice instead of passive voice ('be controlled').

@@ -23,7 +23,7 @@
* Kaniko: builds a container image in Kubernetes without running the build as a privileged container.
* Docker: builds a container image by executing a `docker build` command locally.

The builder type can be controlled by the `builder.type` key in the launch agent config to either `docker`, `kaniko`, or `noop` to disable build. By default, the agent helm chart sets the `builder.type` to `noop`. Additional keys in the `builder` section will be used to configure the build process.
The builder type can be controlled by the `builder.type` key in the launch agent config to either `docker`, `kaniko`, or `noop` to disable build. By default, the agent Helm chart sets the `builder.type` to `noop`. Additional keys in the `builder` section will be used to configure the build process.
Copy link

Choose a reason for hiding this comment

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

⚠️ [vale] reported by reviewdog 🐢
[Google.WordList] Use 'turn off' or 'off' instead of 'disable'.

@@ -23,7 +23,7 @@
* Kaniko: builds a container image in Kubernetes without running the build as a privileged container.
* Docker: builds a container image by executing a `docker build` command locally.

The builder type can be controlled by the `builder.type` key in the launch agent config to either `docker`, `kaniko`, or `noop` to disable build. By default, the agent helm chart sets the `builder.type` to `noop`. Additional keys in the `builder` section will be used to configure the build process.
The builder type can be controlled by the `builder.type` key in the launch agent config to either `docker`, `kaniko`, or `noop` to disable build. By default, the agent Helm chart sets the `builder.type` to `noop`. Additional keys in the `builder` section will be used to configure the build process.
Copy link

Choose a reason for hiding this comment

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

⚠️ [vale] reported by reviewdog 🐢
[Google.Will] Avoid using 'will'.

@@ -23,7 +23,7 @@
* Kaniko: builds a container image in Kubernetes without running the build as a privileged container.
* Docker: builds a container image by executing a `docker build` command locally.

The builder type can be controlled by the `builder.type` key in the launch agent config to either `docker`, `kaniko`, or `noop` to disable build. By default, the agent helm chart sets the `builder.type` to `noop`. Additional keys in the `builder` section will be used to configure the build process.
The builder type can be controlled by the `builder.type` key in the launch agent config to either `docker`, `kaniko`, or `noop` to disable build. By default, the agent Helm chart sets the `builder.type` to `noop`. Additional keys in the `builder` section will be used to configure the build process.
Copy link

Choose a reason for hiding this comment

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

πŸ“ [vale] reported by reviewdog 🐢
[Google.Passive] In general, use active voice instead of passive voice ('be used').

Copy link

github-actions bot commented Oct 3, 2024

Summary

Status Count
πŸ” Total 10
βœ… Successful 9
⏳ Timeouts 0
πŸ”€ Redirected 0
πŸ‘» Excluded 1
❓ Unknown 0
🚫 Errors 0
Full Github Actions output

@ngrayluna
Copy link
Contributor

Closing. Launch is being deprecated.

@ngrayluna ngrayluna closed this Dec 5, 2024
@johndmulhausen johndmulhausen deleted the launch/dockerhub-repo branch January 10, 2025 21:06
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.

4 participants