Hello,
There are a few bugs open in discovery that are the direct result of using
host's edit form when provisioning a discovered host.
There was also a discussion on how discovery will fit in as a host facet.
All the above led to rethinking the inheritance of Host::Base in discovery.
I'm happy that you plan removing STI from discovered hosts. Does that also
mean removing the STI from Host::Managed or that's out of your scope?
One thing I'm not sure I understand from the design regarding facts
> Changes in Facts:
> discovered host will have a connection to DiscoveryFact
> DiscoveryFact will contain:
> key
> value
> value_type
> discovery_facet_id
Does that mean that you'd create a separate Fact class, not using existing
FactName and FactValue?
Thank you
···
--
Marek
On Tuesday 15 of March 2016 17:36:10 Ori Rabin wrote:
Hello,
There are a few bugs open in discovery that are the direct result of using
host’s edit form when provisioning a discovered host.
There was also a discussion on how discovery will fit in as a host facet.
All the above led to rethinking the inheritance of Host::Base in discovery.
> Does that also mean removing the STI from Host::Managed or that's out of
> your scope?
IMHO facets should make it easier for plugins to use Host::Managed with a
proper facet to handle their unique logic instead of inheriting Host::Base.
Once there will be no one to inherit Host::Base, we will be able to merge
its functionality into Host::Managed. That was not my first priority goal,
but it is a nice side effect.
Anyway I don't want to diverge this thread from its original goal, if you
want we can open another one about STI and continue to talk there.
> Does that mean that you'd create a separate Fact class, not using existing
> FactName and FactValue?
We are planning not to reuse current fact tables, since we have facts
before the host is created. We will be able to import those facts to host's
facts storage once the host is provisioned, but I am not sure about the
benefits of such redundancy.
···
On Wednesday, March 16, 2016 at 9:28:29 AM UTC+2, Marek Hulán wrote:
>
> Hello
>
> I'm happy that you plan removing STI from discovered hosts. Does that also
> mean removing the STI from Host::Managed or that's out of your scope?
>
> One thing I'm not sure I understand from the design regarding facts
>
> > Changes in Facts:
> > discovered host will have a connection to DiscoveryFact
> > DiscoveryFact will contain:
> > key
> > value
> > value_type
> > discovery_facet_id
>
> Does that mean that you'd create a separate Fact class, not using existing
> FactName and FactValue?
>
> Thank you
>
> --
> Marek
>
> On Tuesday 15 of March 2016 17:36:10 Ori Rabin wrote:
> > Hello,
> > There are a few bugs open in discovery that are the direct result of
> using
> > host's edit form when provisioning a discovered host.
> > There was also a discussion on how discovery will fit in as a host
> facet.
> > All the above led to rethinking the inheritance of Host::Base in
> discovery.
> >
> > Lzap opened a refactoring ticket with suggestions for changes.
> > After a discussion on irc, Shimon and I created a high level design for
> the
> > more popular suggestion:
> >
> http://projects.theforeman.org/projects/foreman/wiki/Discovered_host_redesig
> > n
> >
> > We would appreciate feedback on this before we start.
> >
> > Thanks,
> > Shimon and Ori
>
>
> From: sshtein@redhat.com
> To: "foreman-dev" <foreman-dev@googlegroups.com>
> Sent: Wednesday, March 16, 2016 3:42:34 AM
> Subject: Re: [foreman-dev] Redesigning discovered host
>
>
>
> > Does that also mean removing the STI from Host::Managed or that's out of
> > your scope?
>
>
> IMHO facets should make it easier for plugins to use Host::Managed with a
> proper facet to handle their unique logic instead of inheriting Host::Base.
> Once there will be no one to inherit Host::Base, we will be able to merge
> its functionality into Host::Managed. That was not my first priority goal,
> but it is a nice side effect.
> Anyway I don't want to diverge this thread from its original goal, if you
> want we can open another one about STI and continue to talk there.
>
>
> > Does that mean that you'd create a separate Fact class, not using existing
> > FactName and FactValue?
>
>
> We are planning not to reuse current fact tables, since we have facts
> before the host is created. We will be able to import those facts to host's
> facts storage once the host is provisioned, but I am not sure about the
> benefits of such redundancy.
The main downside I see to this approach is assuming that facts are
key/value only could cause problems later when you want to consume structured
facts, or perhaps later, have a discovery image based on Ansible instead of
Puppet - why not just use Foreman's existing fact parsing/importing code?
When DiscoveryFacet ends up with a host, you'll have to handle creating
interfaces, so there's another place it'd be a good reason to have native
facts and use the fact parsers.
And just thinking out loud, but why not create the host from the very start? Maybe
it could make sense for a Discovered host to actually show up in the 'All Hosts'
list.
···
----- Original Message -----
On Wednesday, March 16, 2016 at 9:28:29 AM UTC+2, Marek Hulán wrote:
Hello
I’m happy that you plan removing STI from discovered hosts. Does that also
mean removing the STI from Host::Managed or that’s out of your scope?
One thing I’m not sure I understand from the design regarding facts
Changes in Facts:
discovered host will have a connection to DiscoveryFact
DiscoveryFact will contain:
key
value
value_type
discovery_facet_id
Does that mean that you’d create a separate Fact class, not using existing
FactName and FactValue?
Thank you
–
Marek
On Tuesday 15 of March 2016 17:36:10 Ori Rabin wrote:
Hello,
There are a few bugs open in discovery that are the direct result of
using
host’s edit form when provisioning a discovered host.
There was also a discussion on how discovery will fit in as a host
facet.
All the above led to rethinking the inheritance of Host::Base in
discovery.
Lzap opened a refactoring ticket with suggestions for changes.
After a discussion on irc, Shimon and I created a high level design for
the
more popular suggestion:
So if I get it correctly the only reason for not using existing facts is that
FactValue requires host association? I think it should be easy to add a method
to FactName indicating whether this fact requires host or not and then create
new DiscoveredFact subtype which would disable the validation. Btw looking at
code we don't have any validation on host being present, there's just DB
constraint.
···
On Wednesday 16 of March 2016 00:42:34 sshtein@redhat.com wrote:
> > Does that also mean removing the STI from Host::Managed or that's out of
> > your scope?
>
> IMHO facets should make it easier for plugins to use Host::Managed with a
> proper facet to handle their unique logic instead of inheriting Host::Base.
> Once there will be no one to inherit Host::Base, we will be able to merge
> its functionality into Host::Managed. That was not my first priority goal,
> but it is a nice side effect.
> Anyway I don't want to diverge this thread from its original goal, if you
> want we can open another one about STI and continue to talk there.
>
> > Does that mean that you'd create a separate Fact class, not using existing
> > FactName and FactValue?
>
> We are planning not to reuse current fact tables, since we have facts
> before the host is created. We will be able to import those facts to host's
> facts storage once the host is provisioned, but I am not sure about the
> benefits of such redundancy.
–
Marek
On Wednesday, March 16, 2016 at 9:28:29 AM UTC+2, Marek Hulán wrote:
Hello
I’m happy that you plan removing STI from discovered hosts. Does that also
mean removing the STI from Host::Managed or that’s out of your scope?
One thing I’m not sure I understand from the design regarding facts
Changes in Facts:
discovered host will have a connection to DiscoveryFact
DiscoveryFact will contain:
key
value
value_type
discovery_facet_id
Does that mean that you’d create a separate Fact class, not using existing
FactName and FactValue?
Thank you
On Tuesday 15 of March 2016 17:36:10 Ori Rabin wrote:
Hello,
There are a few bugs open in discovery that are the direct result of
using
host’s edit form when provisioning a discovered host.
There was also a discussion on how discovery will fit in as a host
facet.
All the above led to rethinking the inheritance of Host::Base in
discovery.
Lzap opened a refactoring ticket with suggestions for changes.
After a discussion on irc, Shimon and I created a high level design for
>
> And just thinking out loud, but why not create the host from the very
> start? Maybe
> it could make sense for a Discovered host to actually show up in the
> 'All Hosts'
> list.
>
Once we move all functionality except from inventory out of the core host,
it will be a good thing. Actually I was planning on doing exactly that. In
the meantime, we can't create a host object, since it has too many
functional constraints implied.
there's another place it'd be a good reason to have native
> facts and use the fact parsers.
>
As we wrote in the original proposal, we do plan creating the host object
by using current fact importing mechanism. Maybe it's a good reason to vote
against storing those facts in the same storage, since those facts will not
be written by the importer (as it happens with other facts) but it will be
loaded by the discovery plugin.
The main downside I see to this approach is assuming that facts are
> key/value
LZap's crazy idea was storing the facts in their serialized form (as they
come on the wire), which means pure JSON. Our only concern against this
approach is that the search becomes impossible for this storage scheme.
Since the facts have to be serialized in order to be transmitted to foreman
server, we suggested key/value store. I agree that we will need to add
support for hierarchy and arrays, and then it will be JSON-compliant. It
means that is the facts can be serialized as JSON, they will have a place
to be stored be it ansible facts or puppet ones.
···
On Wednesday, March 16, 2016 at 10:01:01 PM UTC+2, stephen wrote:
>
>
>
> ----- Original Message -----
> > From: ssh...@redhat.com
> > To: "foreman-dev" <forem...@googlegroups.com >
> > Sent: Wednesday, March 16, 2016 3:42:34 AM
> > Subject: Re: [foreman-dev] Redesigning discovered host
> >
> >
> >
> > > Does that also mean removing the STI from Host::Managed or that's out
> of
> > > your scope?
> >
> >
> > IMHO facets should make it easier for plugins to use Host::Managed with
> a
> > proper facet to handle their unique logic instead of inheriting
> Host::Base.
> > Once there will be no one to inherit Host::Base, we will be able to
> merge
> > its functionality into Host::Managed. That was not my first priority
> goal,
> > but it is a nice side effect.
> > Anyway I don't want to diverge this thread from its original goal, if
> you
> > want we can open another one about STI and continue to talk there.
> >
> >
> > > Does that mean that you'd create a separate Fact class, not using
> existing
> > > FactName and FactValue?
> >
> >
> > We are planning not to reuse current fact tables, since we have facts
> > before the host is created. We will be able to import those facts to
> host's
> > facts storage once the host is provisioned, but I am not sure about the
> > benefits of such redundancy.
>
> The main downside I see to this approach is assuming that facts are
> key/value only could cause problems later when you want to consume
> structured
> facts, or perhaps later, have a discovery image based on Ansible instead
> of
> Puppet - why not just use Foreman's existing fact parsing/importing code?
>
> When DiscoveryFacet ends up with a host, you'll have to handle creating
> interfaces, so there's another place it'd be a good reason to have native
> facts and use the fact parsers.
>
> And just thinking out loud, but why not create the host from the very
> start? Maybe
> it could make sense for a Discovered host to actually show up in the
> '*All* Hosts'
> list.
>
>
> > On Wednesday, March 16, 2016 at 9:28:29 AM UTC+2, Marek Hulán wrote:
> > >
> > > Hello
> > >
> > > I'm happy that you plan removing STI from discovered hosts. Does that
> also
> > > mean removing the STI from Host::Managed or that's out of your scope?
> > >
> > > One thing I'm not sure I understand from the design regarding facts
> > >
> > > > Changes in Facts:
> > > > discovered host will have a connection to DiscoveryFact
> > > > DiscoveryFact will contain:
> > > > key
> > > > value
> > > > value_type
> > > > discovery_facet_id
> > >
> > > Does that mean that you'd create a separate Fact class, not using
> existing
> > > FactName and FactValue?
> > >
> > > Thank you
> > >
> > > --
> > > Marek
> > >
> > > On Tuesday 15 of March 2016 17:36:10 Ori Rabin wrote:
> > > > Hello,
> > > > There are a few bugs open in discovery that are the direct result of
> > > using
> > > > host's edit form when provisioning a discovered host.
> > > > There was also a discussion on how discovery will fit in as a host
> > > facet.
> > > > All the above led to rethinking the inheritance of Host::Base in
> > > discovery.
> > > >
> > > > Lzap opened a refactoring ticket with suggestions for changes.
> > > > After a discussion on irc, Shimon and I created a high level design
> for
> > > the
> > > > more popular suggestion:
> > > >
> > >
> http://projects.theforeman.org/projects/foreman/wiki/Discovered_host_redesig
> > > > n
> > > >
> > > > We would appreciate feedback on this before we start.
> > > >
> > > > Thanks,
> > > > Shimon and Ori
> > >
> > >
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups
> > "foreman-dev" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an
> > email to foreman-dev...@googlegroups.com .
> > For more options, visit https://groups.google.com/d/optout.
> >
>
Another reason for not using existing facts is saving the type of fact so
we can remove discovery_attributes_set.
We added a table just for 4 facts that are integers to enable searching and
ordering by those facts in the ui.
From what I understood we can now search on integer facts correctly but I'm
not sure that also applies to ordering the columns.
···
On Thu, Mar 17, 2016 at 11:35 AM, Marek Hulán wrote:
Does that also mean removing the STI from Host::Managed or that’s out
of
your scope?
IMHO facets should make it easier for plugins to use Host::Managed with a
proper facet to handle their unique logic instead of inheriting
Host::Base.
Once there will be no one to inherit Host::Base, we will be able to merge
its functionality into Host::Managed. That was not my first priority
goal,
but it is a nice side effect.
Anyway I don’t want to diverge this thread from its original goal, if you
want we can open another one about STI and continue to talk there.
Does that mean that you’d create a separate Fact class, not using
existing
FactName and FactValue?
We are planning not to reuse current fact tables, since we have facts
before the host is created. We will be able to import those facts to
host’s
facts storage once the host is provisioned, but I am not sure about the
benefits of such redundancy.
So if I get it correctly the only reason for not using existing facts is
that
FactValue requires host association? I think it should be easy to add a
method
to FactName indicating whether this fact requires host or not and then
create
new DiscoveredFact subtype which would disable the validation. Btw looking
at
code we don’t have any validation on host being present, there’s just DB
constraint.
–
Marek
On Wednesday, March 16, 2016 at 9:28:29 AM UTC+2, Marek Hulán wrote:
Hello
I’m happy that you plan removing STI from discovered hosts. Does that
also
mean removing the STI from Host::Managed or that’s out of your scope?
One thing I’m not sure I understand from the design regarding facts
Changes in Facts:
discovered host will have a connection to DiscoveryFact
DiscoveryFact will contain:
key
value
value_type
discovery_facet_id
Does that mean that you’d create a separate Fact class, not using
existing
FactName and FactValue?
Thank you
On Tuesday 15 of March 2016 17:36:10 Ori Rabin wrote:
Hello,
There are a few bugs open in discovery that are the direct result of
using
host’s edit form when provisioning a discovered host.
There was also a discussion on how discovery will fit in as a host
facet.
All the above led to rethinking the inheritance of Host::Base in
discovery.
Lzap opened a refactoring ticket with suggestions for changes.
After a discussion on irc, Shimon and I created a high level design
for
> And just thinking out loud, but why not create the host from the very start? Maybe
> it could make sense for a Discovered host to actually show up in the 'All Hosts'
> list.
This is what we are trying to actually avoid. Two main reasons are:
Using Edit Host screen for provisioning new host is a pain. We need to
solve issues like limited editing capabilities when Katello is
installed, redirect URL and stuff like that. Non-host entry allows us to
use New Host form instead of Edit.
Discovering into regular host records leads to limitations, like
duplicate IP or MAC addresses. Users tend to discover hosts and then
reboot them manually into installer trying to create new host record
which often fails.
These issues can be solved, we are just trying to find a better
solution. STI has always been a pain (orchestration layer).
> So if I get it correctly the only reason for not using existing facts is that
> FactValue requires host association? I think it should be easy to add a method
> to FactName indicating whether this fact requires host or not and then create
> new DiscoveredFact subtype which would disable the validation. Btw looking at
> code we don't have any validation on host being present, there's just DB
> constraint.
I was under impression we use foreign key (a gem) and this will not be
possible. If that's technically possible, than we could reuse existing
tables I think. The only limitation is facts are currently strings only,
but we are tracking that under a ticket and perhaps we can fix this as a
part of the feature.
