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

feat: add ai-navigator-app #1514

Merged
merged 12 commits into from
Sep 8, 2023

Conversation

mstruebing
Copy link
Contributor

@mstruebing mstruebing commented Aug 30, 2023

  • Closes D2IQ-98636

What problem does this PR solve?:

This PR adds the ai-navitgator-app to our kommander-applications catalog.

Which issue(s) does this PR fix?:

https://d2iq.atlassian.net/browse/D2IQ-98636

Special notes for your reviewer:

This PR is a WIP.
We need to change the description and the icon.

I've applied these resources to the daily and get:

│ 2023/08/30 09:29:11 licenses.kommander.mesosphere.io is forbidden: User "system:serviceaccount:kommander:default" cannot list resource "licenses" in API group "kommander.mesosphere.io" in the namespace "kommander"                                                                      │

What do I need to do so that the service would be able to read licenses and the corresponding secret?

Also, is it okay to hardcode the namespace to kommander? (this application should always be deployed in this namespace)

Should I split these resources into multiple files or is it okay to have them in one?

I think this needs to be merged into the 2.6 branch to be available in 2.6.1.
I'm not sure how commit messages come into play here, but should I change it to fix?

Is there a way to apply the catalog from this branch on a cluster? Right now I simply applied the Deployment and Service definition, but for a real test I would like to use it like it would be used in reality.

Does this PR introduce a user-facing change?:


Checklist

  • If the PR adds a version bump, ensure there is no breaking change in Licensing model (or NA).
  • If a chart is changed or app configuration is significantly changed, the chart version is correctly incremented (so that apps are not automatically upgraded from a previous version of DKP).

@mstruebing mstruebing self-assigned this Aug 30, 2023
@github-actions github-actions bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 30, 2023
@d2iq-mergebot
Copy link
Contributor

This repo has @d2iq-mergebot integration. You can perform the following commands by submitting a comment. Submit a comment with content "@d2iq-mergebot help" to view more detailed help text and examples. Be sure the have a look at the mergebot documentation, too.For help using mergebot, please refer to the README file here: https://github.com/mesosphere/mergebot/blob/main/README.md
Enabled Mergebot commands:
@d2iq-mergebot test all
@d2iq-mergebot test
@d2iq-mergebot override-status
@d2iq-mergebot help
@d2iq-mergebot backport

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6023072203

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 79.532%

Totals Coverage Status
Change from base Build 6003552572: 0.0%
Covered Lines: 136
Relevant Lines: 171

💛 - Coveralls

@mstruebing mstruebing requested a review from mikolajb August 30, 2023 09:30
@github-actions github-actions bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. services/centralized-grafana services/grafana-logging services/kube-prometheus-stack services/kubecost services/project-grafana-logging services/kommander-ui services/ai-navigator-app and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 4, 2023
@mstruebing mstruebing force-pushed the mstruebing/feat/D2IQ-98636-ai-navigator-app branch from 4168b79 to ab1b448 Compare September 4, 2023 07:54
@mstruebing mstruebing force-pushed the mstruebing/feat/D2IQ-98636-ai-navigator-app branch from 1bd1688 to e9e2f41 Compare September 4, 2023 08:09
@github-actions github-actions bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 4, 2023
@mstruebing mstruebing force-pushed the mstruebing/feat/D2IQ-98636-ai-navigator-app branch from 420cec0 to 63bbca5 Compare September 6, 2023 07:44
@mstruebing mstruebing force-pushed the mstruebing/feat/D2IQ-98636-ai-navigator-app branch from e50743a to 730ae56 Compare September 7, 2023 08:20
@mstruebing mstruebing added ready-for-review ok-to-test Signals mergebot that CI checks are ready to be kicked off labels Sep 7, 2023
@mstruebing mstruebing marked this pull request as ready for review September 7, 2023 08:21
@mstruebing mstruebing force-pushed the mstruebing/feat/D2IQ-98636-ai-navigator-app branch from 56b08bf to f581ab6 Compare September 7, 2023 09:05
@mstruebing
Copy link
Contributor Author

@mikolajb @cbuto is this alright for now?
Limits and requests are currently still missing but I think it would be good to set these after we can have a look at some charts.

Copy link
Contributor

@cbuto cbuto left a comment

Choose a reason for hiding this comment

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

looks good! it would be nice to have tickets for the follow up work just so we don't forget about them. (we should at least set the limits/requests prior to releasing)

@mstruebing mstruebing enabled auto-merge (squash) September 7, 2023 16:10
@cbuto cbuto added the needs-backport Needs to be backported to a release branch label Sep 8, 2023
@mstruebing
Copy link
Contributor Author

looks good! it would be nice to have tickets for the follow up work just so we don't forget about them. (we should at least set the limits/requests prior to releasing)

I've created them yesterday 😇

@cbuto
Copy link
Contributor

cbuto commented Sep 8, 2023

@mstruebing can we backport this to 2.6 as-is?

@mstruebing
Copy link
Contributor Author

@cbuto yes we can! :)

@mstruebing mstruebing merged commit ba3f2f3 into main Sep 8, 2023
33 checks passed
@mstruebing mstruebing deleted the mstruebing/feat/D2IQ-98636-ai-navigator-app branch September 8, 2023 18:36
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

💔 All backports failed

Status Branch Result
release-2.6 There are no commits in this repository touching files in path: "services/logging-operator/4.2.3/pre-upgrade/kustomization.yaml"

Manual backport

To create the backport manually run:

backport --pr 1514

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@cbuto
Copy link
Contributor

cbuto commented Sep 8, 2023

@mstruebing looks like we'll have to manually create the backport by cherry-picking the commit(s) 😞

mstruebing added a commit that referenced this pull request Sep 11, 2023
* feat: add ai-navigator-app

* Closes D2IQ-98636

* chore: use `${releaseNamespace}` instead of hardcoded namespace

* chore: allow to read licenses and secrets

* chore: allow to read licenses and secrets

* chore: add liveness and readiness probes

* chore: update description

* feat: specifiy security context

* chore: use correct logo

* chore: indent securityContext

* chore: add period

* chore: use correct version
@mstruebing mstruebing mentioned this pull request Sep 11, 2023
2 tasks
@mstruebing
Copy link
Contributor Author

@cbuto it's here: #1570

mstruebing added a commit that referenced this pull request Sep 11, 2023
* feat: Introduce empty kustomization of ai-navigation-app (#1547)

* feat: Introduce empty kustomization of ai-navigation-app

Signed-off-by: Mikołaj Baranowski <[email protected]>

* build: Updated .github/service-labeler.yaml

---------

Signed-off-by: Mikołaj Baranowski <[email protected]>
Co-authored-by: d2iq-mergebot <[email protected]>

* feat: add ai-navigator-app (#1514)

* feat: add ai-navigator-app

* Closes D2IQ-98636

* chore: use `${releaseNamespace}` instead of hardcoded namespace

* chore: allow to read licenses and secrets

* chore: allow to read licenses and secrets

* chore: add liveness and readiness probes

* chore: update description

* feat: specifiy security context

* chore: use correct logo

* chore: indent securityContext

* chore: add period

* chore: use correct version

---------

Signed-off-by: Mikołaj Baranowski <[email protected]>
Co-authored-by: Mikołaj Baranowski <[email protected]>
Co-authored-by: d2iq-mergebot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport backport-to-release-2.6 needs-backport Needs to be backported to a release branch ok-to-test Signals mergebot that CI checks are ready to be kicked off ready-for-review services/ai-navigator-app size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants