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

Install Numaflow CRDs as a depenency #71

Merged
merged 1 commit into from
Jun 21, 2024
Merged

Install Numaflow CRDs as a depenency #71

merged 1 commit into from
Jun 21, 2024

Conversation

xdevxy
Copy link
Contributor

@xdevxy xdevxy commented Jun 21, 2024

Modifications

Since Numaplane is managing Numaflow, it is a dependency for sure. This PR includes the installation process with the Numaflow minimal CRDs. If the destination environment already have Numaflow installed then this is just a noop. But it will be helpful for clusters that don't.

Verification

Verified in a local cluster

@xdevxy xdevxy merged commit 3f7eaaf into main Jun 21, 2024
4 checks passed
@xdevxy xdevxy deleted the numaflow_crd branch June 21, 2024 19:34
darshansimha referenced this pull request in darshansimha/numaplane Jun 25, 2024
@juliev0
Copy link
Collaborator

juliev0 commented Jul 19, 2024

@xdevxy Do we need this in our install.yaml? I just noticed that because this is here, and because we brought this file over to numa-manifest-generator, it is trying to install the numaflow CRDs there, which it should not, because that's part of the "numaflow" manifest installation

@xdevxy
Copy link
Contributor Author

xdevxy commented Jul 22, 2024

yeah, after we merged the addon, I think this is no longer needed internally, but this may still needed for open source users. Will this bring any issues if numaplane manifest try to install numaflow CRD again?

@juliev0
Copy link
Collaborator

juliev0 commented Jul 22, 2024

yeah, after we merged the addon, I think this is no longer needed internally, but this may still needed for open source users. Will this bring any issues if numaplane manifest try to install numaflow CRD again?

I definitely think it will create opportunity for error to have this installed by 2 things.

@juliev0
Copy link
Collaborator

juliev0 commented Jul 22, 2024

I guess I do see why you wanted to include it here - the idea was that somebody could basically deploy numaplane and have what they need. But if we do keep it over here, then maybe we should have a separate manifest that gets created which doesn't include them? cc @whynowy

@xdevxy
Copy link
Contributor Author

xdevxy commented Jul 22, 2024

yeah, make sense to separate it out :) do you mind creating an issue for it? Thanks!

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.

3 participants