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

Merge elemental-installer into elemental-operator #7

Closed
mudler opened this issue Jul 5, 2022 · 6 comments
Closed

Merge elemental-installer into elemental-operator #7

mudler opened this issue Jul 5, 2022 · 6 comments
Assignees
Labels
area/teal Elemental Teal

Comments

@mudler
Copy link
Contributor

mudler commented Jul 5, 2022

Merge elemental-installer code from https://github.com/davidcassany/ros-installer/tree/adapt_to_latest_elemental_env_vars (also including those two commits here davidcassany/ros-installer@adapt_to_latest_elemental_env_vars...rancher-sandbox:ros-installer:main ) into the elemental-operator codebase:

  • Port code and also CI workflows
  • Manual QA
  • Update docs
  • Update OBS ( feel free to ping @kkaempf )
@davidcassany
Copy link
Contributor

davidcassany commented Jul 6, 2022

@mudler @kkaempf while digging the code started to have doubt regarding this task. A first I thought we were aiming to an additional installer subcommand in elemental-operator binary, so being mapped to a single package in OBS that produces a single elemental-operator RPM. However we could also simply include the elemental-installer (just rename and similar cosmetic changes) code into the rancher/elemental-operator repository and then add an additional build target for the installer to produce an elemental-installer RPM as an elemental-operator subpackage in OBS.

After seeing rancher/elemental#160 I thought that separate binaries might have more sense. After all, the operator service and the installer a pretty different things to be included into a single binary.

Is this aligned with what you had in mind for this task?

@mudler
Copy link
Contributor Author

mudler commented Jul 6, 2022

Sounds good here 👍 What I had in mind here was to merge codebases, but having still two split binaries, with two separate main files. Didn't thought at the subcommand, but might be just confusing as the operator has no business logic with the installer..

@kkaempf
Copy link
Contributor

kkaempf commented Jul 7, 2022

Yes, we want (and need !) to have two different binaries. elemental-installer runs on the node, elemental-operator on the host cluster. They only have their "API" in common, as the installer needs to communicate with the operator during installation.

The common API is what initiated this card 😉

@davidcassany
Copy link
Contributor

@mudler I am puzzled by the pkg/installer/config.go and the pkg/config/config.go (former ros-installer) Install structs, I assume these are meant to be the same. So elemental-operator creates an installer.Config object which is marshaled and somehow passed to elemental-installer to unmarshal it and run the appropriate elemental-cli command. Is that right?

So my understanding is that pkg/installer/config.go and pkg/config/config.go (former ros-installer) should be merged and shared across the two binaries so we ensure they speak to each other using the same structure. I assume we have plenty of freedom on installer.Config.Elemental.Install syntax (as this is just between elemental-operator and elemental-installer), but I am not sure about the other installer.Config parameters such as installer.SystemAgent and installer.Registration (I understand these are replacement for rancherd related structures in ros-installer).

@davidcassany
Copy link
Contributor

As we noted there was quite some relevant overlap between elemental-operator register command and elemental-installer (former ros-installer), the scope of this card essentially changed on adding the missing pieces to elemental-operator register to fully encompass the required functionality of what was meant to be elemental-installer as it seams to be the fastest path in order to achieve a functional stack. PR #38 is already aligned with this idea, follow up PRs will be required to handle free-from cloud-init data, cleanup unused logic from former ros-installer and any other eventual feature that was we might be missing from ros-installer days.

@davidcassany
Copy link
Contributor

Since basic functional tests were already successful and the scope of this card changed quite a bit after using the elemental-operator register code base I consider this card to be done, essential parts from former ros-installer such as environmental variables mapping are already ported in elemental-operator.

After merging #45 keeping this card around make no sense.

For any missing features in elemental-operator follow up cards are going to be created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/teal Elemental Teal
Projects
Archived in project
Development

No branches or pull requests

3 participants