Project

General

Profile

Bug #4573

Remove unused variables ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata

Added by François ARMAND about 6 years ago. Updated about 6 years ago.

Status:
Released
Priority:
N/A
Category:
System techniques
Target version:
Severity:
User visibility:
Effort required:
Priority:

Description

I'm not sur about the version for which this is the case, it should match the change done for relay servers.

I'm not sur about ADMIN, I believe it was already removed in 2.10.


Subtasks

Bug #4574: Remove ADMIN from common metadataRejected2014-03-07Nicolas CHARLESActions
Bug #4575: Remove unused datas from the LDAP tree initialisationReleased2014-03-07François ARMANDActions
Bug #4576: Remove ADMIN from the LDAP treeReleased2014-03-07François ARMANDActions

Related issues

Related to Rudder - User story #4033: Add system variables filling MANAGED_NODES_NAME and MANAGED_NODES_ID in RudderReleased2013-10-09François ARMANDActions
#1

Updated by Nicolas CHARLES about 6 years ago

POLICYCHILDREN and CHILDRENID were removed in 2.8, ADMIN in 2.10.
I'm openin another ticket for ADMIN

#2

Updated by Nicolas CHARLES about 6 years ago

  • Target version set to 2.8.4
#3

Updated by Nicolas CHARLES about 6 years ago

  • Status changed from 8 to Pending technical review
  • Assignee changed from Nicolas CHARLES to Jonathan CLARKE
  • Pull Request set to https://github.com/Normation/rudder-techniques/pull/312
#4

Updated by Nicolas CHARLES about 6 years ago

  • Status changed from Pending technical review to In progress

ha, it is a bit more complex than that, as it fails at deployment

#5

Updated by Nicolas CHARLES about 6 years ago

  • Assignee changed from Jonathan CLARKE to Nicolas CHARLES
#6

Updated by Nicolas CHARLES about 6 years ago

  • Status changed from In progress to Pending technical review
  • Assignee changed from Nicolas CHARLES to Jonathan CLARKE

Ok, it doesn't fail, it just complains in the log that some variables cannot be stored, but it works

#7

Updated by Jonathan CLARKE about 6 years ago

  • Status changed from Pending technical review to Discussion
  • Assignee changed from Jonathan CLARKE to François ARMAND

I don't understand this ticket. Removing stuff is not a bug. What is the actual bug (what doesn't work or is broken here?). Please clarify.

#8

Updated by François ARMAND about 6 years ago

Having dead code is a bug:

- it may be call in unwanted or unknown way, leading to bigger problem (security pb) and/or extremely hard bug to reproduce
- it may send bad message to future mainteners, thinking that something is used but is actually not.

That's of course generalities, but in that precise case, these variables were still present in technique metadata but not used for the promise generation, so we add a desynchronisation between technique metadata and actual variable, and we were paying a non negligible processing time for them for nothing.

Moreover, they were the only remaining use cases of the ${rudder.ruleid.target.XXX} parameter kind, blocking a possible major simplification (see #4585)

#9

Updated by François ARMAND about 6 years ago

  • Status changed from Discussion to Pending technical review
  • Assignee changed from François ARMAND to Jonathan CLARKE
#10

Updated by Jonathan CLARKE about 6 years ago

  • Status changed from Pending technical review to Discussion
  • Assignee changed from Jonathan CLARKE to François ARMAND

François ARMAND wrote:

Having dead code is a bug:

- it may be call in unwanted or unknown way, leading to bigger problem (security pb) and/or extremely hard bug to reproduce
- it may send bad message to future mainteners, thinking that something is used but is actually not.

That's of course generalities, but in that precise case, these variables were still present in technique metadata but not used for the promise generation, so we add a desynchronisation between technique metadata and actual variable, and we were paying a non negligible processing time for them for nothing.

Moreover, they were the only remaining use cases of the ${rudder.ruleid.target.XXX} parameter kind, blocking a possible major simplification (see #4585)

Thanks for the clarification.

I agree this needs removing, however, in the interest of stability, we must avoid making unnecessary changes, and in particular any changes that don't fix a problem suffered by users in released branches. Can we retarget this to 2.10?

#11

Updated by François ARMAND about 6 years ago

  • Status changed from Discussion to Pending technical review
  • Assignee changed from François ARMAND to Jonathan CLARKE

Well, they should have been removed along with the introduction of MANAGED_NODES_NAME and MANAGED_NODES_ID in #4033 (so in 2.8), but I'm ok to remove them only in 2.10.

#12

Updated by Jonathan CLARKE about 6 years ago

  • Status changed from Pending technical review to Discussion
  • Assignee changed from Jonathan CLARKE to Nicolas CHARLES

François ARMAND wrote:

Well, they should have been removed along with the introduction of MANAGED_NODES_NAME and MANAGED_NODES_ID in #4033 (so in 2.8), but I'm ok to remove them only in 2.10.

Great. Assigning to Nicolas to retarget the PR.

#13

Updated by Jonathan CLARKE about 6 years ago

François ARMAND wrote:

Having dead code is a bug:

Oh, and to respond to this, let me clarify what my problem here is: maybe having dead code is a bug, but the title of this ticket never mentions dead code. It is expressed exactly like a "Let's remove some stuff" ticket which sounds scary, whereas it should be more like a "Let's clean up this unused stuff" which sounds good.

Titles matter. A lot.

#14

Updated by François ARMAND about 6 years ago

  • Target version changed from 2.8.4 to 2.10.0~beta1
#15

Updated by François ARMAND about 6 years ago

  • Status changed from Discussion to In progress

Nico, could you retarget the associated PR (and other for the children) to 2.10 (given all the discussion above ?)

Thanks !

#16

Updated by Nicolas CHARLES about 6 years ago

  • Pull Request changed from https://github.com/Normation/rudder-techniques/pull/312 to https://github.com/Normation/rudder-techniques/pull/317
#17

Updated by Jonathan CLARKE about 6 years ago

  • Subject changed from Remove ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata to Remove unusued variables ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata

Clarifying the title to make it actually sound like a bug!

#18

Updated by Nicolas CHARLES about 6 years ago

  • Status changed from In progress to Pending release
  • % Done changed from 67 to 100

Applied in changeset policy-templates:commit:6f23559579946857d0bda5fd6c252e926f4c264d.

#19

Updated by Anonymous about 6 years ago

Applied in changeset policy-templates:commit:cec1a3bd22ad95414386b2b935fdf1a8dc37eb16.

#20

Updated by Vincent MEMBRÉ about 6 years ago

  • Subject changed from Remove unusued variables ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata to Remove unused variables ADMIN, POLICYCHILDREN and CHILDRENID from "common" system technique metadata
#21

Updated by Vincent MEMBRÉ about 6 years ago

  • Status changed from Pending release to Released

This bug has been fixed in Rudder 2.10.0~beta1, which was released today.
Check out:

The release announcement: http://www.rudder-project.org/pipermail/rudder-announce/2014-March/000084.html
The full ChangeLog: http://www.rudder-project.org/foswiki/bin/view/System/Documentation:ChangeLog210
Download information: https://www.rudder-project.org/site/get-rudder/downloads/

Also available in: Atom PDF