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

Gitea updates #77

Merged
merged 3 commits into from
Aug 29, 2023
Merged

Gitea updates #77

merged 3 commits into from
Aug 29, 2023

Conversation

adetalhouet
Copy link
Member

@adetalhouet adetalhouet commented Aug 22, 2023

This PR introduce three changes:

  • update documentation to provide steps for deployment on OpenShift (if there is a way to include condition in kpt deployment, I'd rather have these CRs included in the package directly).
  • Add gitea postgresql and gitea secret to kpt package: given the configuration is already hardcoded in the configuration, as defined here, there is no point in asking the user to create these objects.
  • the package is missing the namespace definition, hence when applying it using the following commands
     kpt pkg get --for-deployment https://github.com/nephio-project/nephio-example-packages.git/gitea@v1.0.1
     kpt fn render gitea/
     kpt live init gitea/
     kpt live apply gitea/ --reconcile-timeout 15m --output=table
    
    it fails with
    error: task failed (action: "Inventory", name: "inventory-add-0"): namespaces "gitea" not found`
    

Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>
@nephio-prow
Copy link
Contributor

nephio-prow bot commented Aug 22, 2023

Hi @adetalhouet. Thanks for your PR.

I'm waiting for a nephio-project member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>
@adetalhouet adetalhouet changed the title Add missing gitea namespace Gitea updates Aug 23, 2023
Given the configuration is already hardcoded in the configuration,
as defined [here](https://github.com/nephio-project/nephio-example-packages/blob/main/gitea/secret-gitea-inline-config.yaml#L21-L22), there is no point
in asking the user to create these objects.

Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>
@johnbelamaric
Copy link
Member

/ok-to-test

@@ -34,4 +37,73 @@ cat <<EOF | kubectl apply -f -
username: ...
password: ...
EOF
```

# OpenShift
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. This is OK for now. But it gets at a general problem we haven't solved yet and we are likely to see over and over again.

Essentially, deploying this package into a particular environment creates the need to apply some additional resources. This could be treated as a separate, dependent package.

We should think about is whether we should have some sort of "deployment context" resource that our different packages understand, or a an "environment". This could be done as a binding of sorts, like described in kptdev/kpt#3973. If you bind the "openshift" resource to it, it would pull these in as a package dependency or perhaps as some sort of "mix in" concept we haven't yet figured out.

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 an alternative before we figure this out is to just use a derivative package.

We can clone the current Gitea package into a new GCP Gitea package and an OpenShift Gitea package, and add the appropriate resources.

@johnbelamaric
Copy link
Member

  • update documentation to provide steps for deployment on OpenShift (if there is a way to include condition in kpt deployment, I'd rather have these CRs included in the package directly).

Only way to do this right now is with a function, but I think the better option is the binding/injection/dependency/mix in concept I mentioned in another comment. We should work on that.

  • Add gitea postgresql and gitea secret to kpt package: given the configuration is already hardcoded in the configuration, as defined here, there is no point in asking the user to create these objects.

I think that's fine for now. We have nephio-project/nephio#346 to address secrets management in general. It becomes another dependency in the BYOC flow; for the sandbox maybe we use Vault.

@johnbelamaric
Copy link
Member

@adetalhouet For the GKE case, I removed the metallb annotation and added networking.gke.io/load-balancer-type: "Internal" to that the service stays internal. Not sure if you need something similar in open shift.

@johnbelamaric
Copy link
Member

/approve

@nephio-prow
Copy link
Contributor

nephio-prow bot commented Aug 23, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnbelamaric

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@nephio-prow nephio-prow bot added the approved label Aug 23, 2023
@adetalhouet
Copy link
Member Author

@adetalhouet For the GKE case, I removed the metallb annotation and added networking.gke.io/load-balancer-type: "Internal" to that the service stays internal. Not sure if you need something similar in open shift.

Well, I haven't actually looked into this. But with OpenShift, I believe we wouldn't need metallb altogether. If the gitea service needs to be expose, it would have a Route (similar to Ingress), which would provide a FQDN for the service.

@johnbelamaric
Copy link
Member

  • Add gitea postgresql and gitea secret to kpt package: given the configuration is already hardcoded in the configuration, as defined here, there is no point in asking the user to create these objects.

I think those in the "inline" config get overwritten by whatever is in the secrets.

@johnbelamaric
Copy link
Member

Anyway, let's merge this for now but I think we will want to clean up if we create the nephio-standard-packages repository.

@johnbelamaric
Copy link
Member

/lgtm

@nephio-prow nephio-prow bot added the lgtm label Aug 29, 2023
@nephio-prow nephio-prow bot merged commit 6c95409 into nephio-project:main Aug 29, 2023
adetalhouet added a commit to adetalhouet/nephio-example-packages that referenced this pull request Sep 25, 2023
* Add missing gitea namespace

Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>

* Add doc for OpenShift

Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>

* Add gitea postgresql and gitea secret to kpt package

Given the configuration is already hardcoded in the configuration,
as defined [here](https://github.com/nephio-project/nephio-example-packages/blob/main/gitea/secret-gitea-inline-config.yaml#L21-L22), there is no point
in asking the user to create these objects.

Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>

---------

Signed-off-by: Alexis de Talhouët <adetalhouet89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants