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

bm/ovirt: pass API_VIP/DNS_VIP from cluster config/installer and render templates with it #812

Closed
rgolangh opened this issue Jun 3, 2019 · 22 comments · Fixed by #943
Closed
Labels

Comments

@rgolangh
Copy link
Contributor

rgolangh commented Jun 3, 2019

I'm developing the platform provider for ovirt #766, which is a platform which doesn't provide a DNS service, and as such it utilzes the work done on metal3. To bootstrap a working DNS solution, the installer asks the user for an IP to be the virtual IP of the DNS - DNS_VIP.
This DNS_VIP is a value which must end up in the masters ignition files for the ovirt platform, and possibly others.

Today I see that there are number of files which bootstrap MCO, one of them is config-file:

bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.configFile, "config-file", "", "ClusterConfig ConfigMap file.")

Which is the serialized InstallConfig of the installer, which has all I need:
https://github.com/openshift/installer/blob/479ca0fc0fd46c489c4b97d783bf1dd58cc373a2/pkg/types/installconfig.go#L42

What I want is to add this ClusterConfigMap to the RenderConfig and by that I could render my templates.
Can I rely on the installer InstallConfig type and to serialize the content to it and put it in render config? It creates dependency on the installer types, and I don't know yet how does it fit the overall design of those components.

@kikisdeliveryservice
Copy link
Contributor

Briefly looking through, it looks like the following prs would need to land before we'd tackle this:
openshift/api#314
#767

What is the status of the first PR in openshift/api ??

@rgolangh
Copy link
Contributor Author

rgolangh commented Jun 3, 2019

I have nothing to change there, just waiting for approval

@kikisdeliveryservice kikisdeliveryservice changed the title Operator should consume cluster config map, and render templates with it ovirt: Operator should consume cluster config map, and render templates with it Jun 3, 2019
@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Jun 3, 2019

specifically which values are you needing from the cluster config? Networking? The mco already consumes the install config, so if you could be more specific about what you need from that it would be helpful. 😃

@kikisdeliveryservice kikisdeliveryservice changed the title ovirt: Operator should consume cluster config map, and render templates with it ovirt: pass DNS_VIP from cluster config/installer and render ovirt templates with it Jun 3, 2019
@kikisdeliveryservice
Copy link
Contributor

Also if you can add more details about the platform, that would be great as I don't think we have that much familiarity with it.

@rgolangh
Copy link
Contributor Author

rgolangh commented Jun 3, 2019

You'll find it here, under the new 'ovirt' platform (not merged yet obviously)
https://github.com/rgolangh/installer/blob/fd85cbdb57e9d3e5ba20bba757468c9b8c57642d/pkg/types/ovirt/platform.go#L27

@rgolangh
Copy link
Contributor Author

rgolangh commented Jun 3, 2019

I can think of more data which I need to share, like the the platform authentication details, which will be used by the cluster-api or actuator to provision machines

@kikisdeliveryservice
Copy link
Contributor

can you outline what PRs need to merge / the overall status of this ovirt platform? before this PR could even merge?

@runcom
Copy link
Member

runcom commented Jun 3, 2019

What I want is to add this ClusterConfigMap to the RenderConfig and by that I could render my templates.

we're Rendering using the cluster config already - what's the reason to be in RenderConfig as well? you might want it in ControllerConfig maybe (still looking at the code and the related ovirt PRs)

@rgolangh
Copy link
Contributor Author

rgolangh commented Jun 3, 2019

RenderConfig is the one that is used to execute templating on templates/master/... no?

@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Jun 3, 2019

Ah so I didn't understand that this is actually speculative and that the installer does not yet even support ovirt. You'd first need to get that installer config PR merged before the MCO can do anything with your new data type. And like @runcom said the mco already consumes the clusterconfig so that bit of general work would not be needed

@abhinavdahiya
Copy link
Contributor

Installer team is deprecating all clients of the InstallConfig in the cluster ie. ClusterConfig ConfigMap. Please move to infrastructures.config.openshift.io.

@runcom
Copy link
Member

runcom commented Jun 3, 2019

Please move to infrastructures.config.openshift.io.

cool, we do play with that already as well in code (iirc, can't check right now).

@rgolangh
Copy link
Contributor Author

rgolangh commented Jun 4, 2019

@kikisdeliveryservice speculative? no. Its a work in progress toward making it happen.
For the installer to support ovirt, or almost any platform, I must to have MCO support, its integral, but yes the patch to at least getting it into the api platform type is the start.

@runcom got any reference to infrastructures.config.openshift.io?

@runcom runcom added the jira label Jun 4, 2019
@bcrochet
Copy link
Member

bcrochet commented Jun 4, 2019

I'd like to add that the Baremetal platform also needs some/all of these values as well. That is more pressing as we are trying to get Baremetal support into 4.2. I currently have PR #795 that adds some static pod definitions that would need at a minimum the API vip and the DNS vip. Should I open a separate issue for that? @kikisdeliveryservice

@runcom
Copy link
Member

runcom commented Jun 4, 2019

I currently have PR #795 that adds some static pod definitions that would need at a minimum the API vip and the DNS vip. Should I open a separate issue for that?

if it's tackling the same issue, let's generalize this issue.

@kikisdeliveryservice
Copy link
Contributor

Yes I'd say just use this issue. Easier to keep track.

@kikisdeliveryservice kikisdeliveryservice changed the title ovirt: pass DNS_VIP from cluster config/installer and render ovirt templates with it bm/ovirt: pass API_VIP/DNS_VIP from cluster config/installer and render templates with it Jun 4, 2019
@rgolangh
Copy link
Contributor Author

rgolangh commented Jun 4, 2019

RenderConfig is the one that is used to execute templating on templates/master/... no?

I see that the controller bootstrap does that:
cmd/machine-config-controller/bootstrap.go#runbootstrapCmd

So this means I won't have access the cluster config

@runcom
Copy link
Member

runcom commented Jun 4, 2019

@runcom got any reference to infrastructures.config.openshift.io?

@rgolangh we're playing with that in pkg/operator but I'm not sure how that's handled outside the MCO to populate it:

	infra, err := optr.infraLister.Get("cluster")
	if err != nil {
		return nil, nil, err
	}

I believe it's setup in the installer 🤔

@rgolangh
Copy link
Contributor Author

rgolangh commented Jun 6, 2019

Installer team is deprecating all clients of the InstallConfig in the cluster ie. ClusterConfig ConfigMap. Please move to infrastructures.config.openshift.io.

@abhinavdahiya I don't know what this namespace refer to - I saw you mentioning this several times - can you please explain this?
If I get you correctly and following @runcom comment, If I want to shar edata from the installer then I should exapand InfrastructureStatus struct?
Or will InstallConfig will be under InfrastructureStatus?

@celebdor
Copy link
Contributor

celebdor commented Jun 6, 2019

Issue explaining the move of installconfig stuff to infrastructures.config.openshift.io openshift/installer#680

@celebdor
Copy link
Contributor

celebdor commented Jun 6, 2019

@rgolangh
Copy link
Contributor Author

@runcom I think we can link #943 with this issue and edit its name - what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants