feat: introduces features fluent interface #668

bartoszmajsak · 2023-10-25T18:56:09Z

Description

Extracts the fluent interface for Features from PR #605. This allows other components to configure cluster resources using this apprach before the original PR gets merged.

No changes to the reconcile logic have been introduced.

How Has This Been Tested?

Merge criteria:

The commits are squashed in a cohesive manner and have meaningful messages.
Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
The developer has manually tested the changes and verified that the changes work

Extracts the fluent interface for Features from PR opendatahub-io#605. This allows other components to configure cluster resources using this interface before the original PR gets merged. No changes to the reconcile logic have been introduced.

bartoszmajsak · 2023-10-25T18:59:41Z

pkg/feature/templates/namespace.patch.tmpl

This file exists solely to make //go:embed happy. As #605 is using templates I thought I would make this part available as the "core" functionality so it's easier for others to use if necessary. But can be deleted if e.g. @israel-hdez does not need it yet.

Most likely, I'll need the folder.

I've seen other projects (which I no longer remember), use a .keep empty file to force git to keep the folder. It is just a suggestion. I'm file keeping this unused template with the same goal.

I tried with .gitkeep but //go:embed fails if you have only such files. Thus the template. I should have mention it earlier :)

israel-hdez

I only quickly scanned the code, since I've already read the code at #605.

It seems to contain the baseline that we can use for minimal OSSM and Serverles installation, as prereqs for KServe.

components/dashboard/dashboard.go

components/workbenches/workbenches.go

israel-hdez · 2023-10-27T21:56:59Z

pkg/feature/templates/namespace.patch.tmpl

Most likely, I'll need the folder.

I've seen other projects (which I no longer remember), use a .keep empty file to force git to keep the folder. It is just a suggestion. I'm file keeping this unused template with the same goal.

pkg/feature/resources.go

pkg/feature/raw_resources.go

zdtsw · 2023-10-30T16:52:52Z

pkg/feature/feature.go

+	return nil
+}
+
+func convertToRFC1123Subdomain(input string) string {


not sure i understand why need this

Each feature is named using just a string, see here for example.

In order to have this in objectMeta.Name we have to convert it to follow this RFC (it simply replaces non-alphanumeric characters with a hyphen), otherwise, it will error complaining about wrong naming.

Thanks for bringing it up, as I realized this implementation doesn't really ensure all of it. It should make sure that the name:

contain no more than 63 characters

contain only lowercase alphanumeric characters, '-' or '.'

start with an alphanumeric character

end with an alphanumeric character

Using the example above this object will be named "opendatahub-create-service-mesh-routing-resources-for-dashboard" if opendatahub is app namespace. This way it's easy to see which tracker object is used for where and by which feature.

If you agree this is the right naming approach I will enhance the logic to fulfill all the points above.

Reworked in 3f5efbd

ok, i see.
guess the string is just something meaningful to represent the component and servicemesh?
if the limitation of 63 chars from k8s will be applied, just thinking to have something can be useful for ODH (default namespace opendatahub" as 10 chars) and downstream (default namespace 21 chars)
create-service-mesh-routing-resources-for-dashboard (44 chars) will work in ODH, but not downstream

guess the string is just something meaningful to represent the component and servicemesh?

well... it's the name which should somewhat reveal it's reason to exist :) we might want to extend the .Spec to have .AppNamespace instead but I'm not quite sure.

create-service-mesh-routing-resources-for-dashboard (44 chars) will work in ODH, but not downstream

it will be truncated to 63 chars, I guess that is fine at this point. WDYT?

yeah, i saw the new test cases you added there. just need to be mindful when we set the value later on.
we should make it something like "dashboard-create-service-mesh-routing-resources", so the important part "dashboard" wont be truncated

That's really a great point! That got me thinking that maybe .Spec should have extra fields such as Component, Namespace and thus be more useful. I will think a bit more and provide improvements as a separate chunk if that works for you.

VaishnaviHire · 2023-10-30T17:17:53Z

/test opendatahub-operator-e2e

- adds additional conditions - adds tests

it is a leftover from KfDef plugin code

zdtsw · 2023-10-31T11:48:02Z

should update https://github.com/maistra/opendatahub-operator/blob/features_dsl/Makefile#L66 to new version?

bartoszmajsak · 2023-10-31T15:10:28Z

should update maistra/opendatahub-operator@features_dsl/Makefile#L66 to new version?

It's the same in the target branch

opendatahub-operator/Makefile

Line 66 in c9aa358

OPERATOR_SDK_VERSION ?= v1.24.1

It might be that my local operator-sdk from other project took precedence here, as Makefile does this:

OPERATOR_SDK = $(shell which operator-sdk)

but in my case it returns:

❯ which operator-sdk
~/.asdf/shims/operator-sdk

zdtsw · 2023-11-01T07:26:26Z

should update maistra/opendatahub-operator@features_dsl/Makefile#L66 to new version?

It's the same in the target branch

opendatahub-operator/Makefile

Line 66 in c9aa358

OPERATOR_SDK_VERSION ?= v1.24.1

It might be that my local operator-sdk from other project took precedence here, as Makefile does this:
OPERATOR_SDK = $(shell which operator-sdk)
but in my case it returns:
❯ which operator-sdk
~/.asdf/shims/operator-sdk

so, we still want to use 1.24.1, right? if so, we will not need the changes e.g bundle.Dockerfile

zdtsw

/lgtm

zdtsw · 2023-11-01T07:39:58Z

tested in cluster with the build. components are working.

bartoszmajsak · 2023-11-01T08:31:35Z

should update maistra/opendatahub-operator@features_dsl/Makefile#L66 to new version?

It's the same in the target branch

opendatahub-operator/Makefile

Line 66 in c9aa358

OPERATOR_SDK_VERSION ?= v1.24.1

It might be that my local operator-sdk from other project took precedence here, as Makefile does this:
OPERATOR_SDK = $(shell which operator-sdk)
but in my case it returns:
❯ which operator-sdk
~/.asdf/shims/operator-sdk
so, we still want to use 1.24.1, right? if so, we will not need the changes e.g bundle.Dockerfile

@zdtsw I will address both in the follow-up PR.

zdtsw · 2023-11-01T08:33:02Z

/lgtm

add approval based on new commit is after rebase from incubation branch

zdtsw · 2023-11-01T10:17:59Z

/approved

openshift-ci · 2023-11-01T10:18:46Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: zdtsw

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:

OWNERS

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

zdtsw · 2023-11-03T08:09:18Z

ref #666

Extracts the fluent interface for Features from PR opendatahub-io#605. This allows other components to configure cluster resources using this approach before the original PR gets merged. (cherry picked from commit adb6658)

feat: introduces features fluent interface

73c7ec9

Extracts the fluent interface for Features from PR opendatahub-io#605. This allows other components to configure cluster resources using this interface before the original PR gets merged. No changes to the reconcile logic have been introduced.

bartoszmajsak requested review from zdtsw and VaishnaviHire October 25, 2023 18:56

openshift-ci bot requested a review from etirelli October 25, 2023 18:56

bartoszmajsak commented Oct 25, 2023

View reviewed changes

israel-hdez reviewed Oct 27, 2023

View reviewed changes

bartoszmajsak added 3 commits October 30, 2023 13:43

feat(cert): self-signed cert allows wildcards

163104a

fix(import): rearranges import

d096c79

fix(bundle): regenerates manifests after refactoring

8328f0e

zdtsw reviewed Oct 30, 2023

View reviewed changes

bartoszmajsak added 4 commits October 30, 2023 18:49

Merge branch 'incubation' into features_dsl

a937c28

Merge branch 'incubation' into features_dsl

337d407

chore: moves RFC1123 logic to common

3f5efbd

- adds additional conditions - adds tests

fix: removes NameValue overwrite option

8df15cb

it is a leftover from KfDef plugin code

bartoszmajsak added 3 commits October 31, 2023 16:43

chore: reuses cluster.CreateNamespace for feature-owned ns

7e260c6

feat: introduces initializer to handle features creation

7092074

Merge branch 'incubation' into features_dsl

ba4a18b

zdtsw approved these changes Nov 1, 2023

View reviewed changes

openshift-ci bot assigned zdtsw Nov 1, 2023

openshift-ci bot added the lgtm label Nov 1, 2023

Merge branch 'incubation' into features_dsl

a3c13ef

openshift-ci bot removed the lgtm label Nov 1, 2023

openshift-ci bot added the lgtm label Nov 1, 2023

zdtsw added the approved label Nov 1, 2023

bartoszmajsak merged commit adb6658 into opendatahub-io:incubation Nov 1, 2023
6 of 7 checks passed

bartoszmajsak mentioned this pull request Nov 1, 2023

Service Mesh SSO(Auth) Integration opendatahub-io/opendatahub-community#97

Open

heyselbi mentioned this pull request Nov 1, 2023

New Manifests for required CRs installation opendatahub-io/kserve#105

Closed

bartoszmajsak mentioned this pull request Nov 2, 2023

fix(make): ensures projects version of operator-sdk is used #695

Merged

3 tasks

VaishnaviHire mentioned this pull request Nov 15, 2023

Sync 2.5 release red-hat-data-services/rhods-operator#116

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduces features fluent interface #668

feat: introduces features fluent interface #668

bartoszmajsak commented Oct 25, 2023

bartoszmajsak Oct 25, 2023 •

edited

israel-hdez Oct 27, 2023

bartoszmajsak Oct 28, 2023

israel-hdez left a comment

israel-hdez Oct 27, 2023

zdtsw Oct 30, 2023

bartoszmajsak Oct 30, 2023 •

edited

bartoszmajsak Oct 31, 2023

zdtsw Oct 31, 2023

bartoszmajsak Oct 31, 2023

zdtsw Oct 31, 2023

bartoszmajsak Oct 31, 2023

VaishnaviHire commented Oct 30, 2023

zdtsw commented Oct 31, 2023

bartoszmajsak commented Oct 31, 2023

zdtsw commented Nov 1, 2023 •

edited

zdtsw left a comment

zdtsw commented Nov 1, 2023 •

edited

bartoszmajsak commented Nov 1, 2023

zdtsw commented Nov 1, 2023 •

edited

zdtsw commented Nov 1, 2023

openshift-ci bot commented Nov 1, 2023

zdtsw commented Nov 3, 2023

feat: introduces features fluent interface #668

feat: introduces features fluent interface #668

Conversation

bartoszmajsak commented Oct 25, 2023

Description

How Has This Been Tested?

Merge criteria:

bartoszmajsak Oct 25, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

israel-hdez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bartoszmajsak Oct 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VaishnaviHire commented Oct 30, 2023

zdtsw commented Oct 31, 2023

bartoszmajsak commented Oct 31, 2023

zdtsw commented Nov 1, 2023 • edited

zdtsw left a comment

Choose a reason for hiding this comment

zdtsw commented Nov 1, 2023 • edited

bartoszmajsak commented Nov 1, 2023

zdtsw commented Nov 1, 2023 • edited

zdtsw commented Nov 1, 2023

openshift-ci bot commented Nov 1, 2023

zdtsw commented Nov 3, 2023

bartoszmajsak Oct 25, 2023 •

edited

bartoszmajsak Oct 30, 2023 •

edited

zdtsw commented Nov 1, 2023 •

edited

zdtsw commented Nov 1, 2023 •

edited

zdtsw commented Nov 1, 2023 •

edited