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

[hostcfgd] wrong behavior for disable_feature #15

Open
wenyiz2021 opened this issue Aug 26, 2022 · 1 comment
Open

[hostcfgd] wrong behavior for disable_feature #15

wenyiz2021 opened this issue Aug 26, 2022 · 1 comment
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@wenyiz2021
Copy link

wenyiz2021 commented Aug 26, 2022

In enable_feature:

        for suffix in feature_suffixes:
            cmds.append("sudo systemctl unmask {}.{}".format(feature_name, suffix))
        cmds.append("sudo systemctl enable {}.{}".format(feature_name, feature_suffixes[-1]))
        cmds.append("sudo systemctl start {}.{}".format(feature_name, feature_suffixes[-1]))

While in disable_feature, this iteration looks wrong:

        for suffix in reversed(feature_suffixes):
            cmds.append("sudo systemctl stop {}.{}".format(feature_name, suffix))
            cmds.append("sudo systemctl disable {}.{}".format(feature_name, feature_suffixes[-1]))
            cmds.append("sudo systemctl mask {}.{}".format(feature_name, feature_suffixes[-1]))

My idea is to change to:

        for suffix in reversed(feature_suffixes):
            cmds.append("sudo systemctl stop {}.{}".format(feature_name, suffix))
            cmds.append("sudo systemctl disable {}.{}".format(feature_name, suffix))
            cmds.append("sudo systemctl mask {}.{}".format(feature_name, suffix))

Let's say,
telemetry has feature_suffixes = ['service', 'timer']

enable_feature:

sudo systemctl unmask t.service
sudo systemctl unmask t.timer

sudo systemctl enable t.timer   
sudo systemctl start t.timer

=> ok

disable_feature:

sudo systemctl stop t.timer
sudo systemctl disable t.timer
sudo systemctl mask t.timer

sudo systemctl stop t.service
sudo systemctl disable t.timer  ?? why do it again
sudo systemctl mask t.timer ??

With new change:
disable:

sudo systemctl stop t.timer
sudo systemctl disable t.timer
sudo systemctl mask t.timer

sudo systemctl stop t.service
sudo systemctl disable t.service
sudo systemctl mask t.service

dhcp has [service]

enable:

sudo systemctl unmask d.service	
sudo systemctl enable d.service
sudo systemctl start d.service

=>ok
disable:

sudo systemctl stop d.service
sudo systemctl disable d.service  
sudo systemctl mask d.service 

=>ok

@wenyiz2021
Copy link
Author

wenyiz2021 commented Aug 26, 2022

@yozhao101 @yxieca for viz
there is unit test protect current behavior, so PR check will fail if making any change

@wenyiz2021 wenyiz2021 added bug Something isn't working help wanted Extra attention is needed labels Sep 7, 2022
aidan-gallagher pushed a commit to aidan-gallagher/sonic-host-services that referenced this issue Nov 16, 2024
* first cut

* first cut lib code with unit test

* save it just in case, as VM under risk

* partial update, as vm at risk

* intermediate saving

* First run code complete

* self review update

* partial compile

* In middle of compilation

* compiled OK

* common ut passes

* events_service unit tests complete

* test code in progress

* evens publish covere by unit test

* events 75% covered by UT

* subscribe UT done

* Drop internal readme

* Update upon self review - mostly on comments

* More on comments update

* minor fix of copy/paste error

* Comments update

* more comments

* minor updates; merged with master via shared

* Per review comments

* minor updates

* minor: name change for a typedef

* Added EXIT code

* Minor name change; Enabled py build for events

* Minor signature update to adapt to SWIG generated python

* Made zmq send & receive thread safe

* temp commit to enable merge

* removed hacks; Added set log level API

* Minor signature change

* Added unit test for C wrap

* Corrected per review comments; Addede log message for published events

* minor updates;no logical code changes

* corrected per review comments; Added accidentally removed test

* Added back accidentally removed test code file

* retire runtime id on deinit

* restore accidental removal

* Switched to shared_ptr per review comments

* minor non logical code changes

* fix syntax

* few follow up changes for bare ptr to shared ptr conversion

* send event as JSON string

* syntax

* syntax

* syntax

* syntax

* syntax

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* changing wrap signature to use struct instead of JSON string

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* compile fix

* Add libzmq5 as dependency for ubuntu-20.04 build (sonic-net#7)

Co-authored-by: Ubuntu <zain@zb-dev-vm.022x1jpnpm4u1iy2d325acts3c.yx.internal.cloudapp.net>

* Modify azp (sonic-net#8)

* Add libzmq5 as dependency for ubuntu-20.04 build

* Add libzmq3-dev dependency to build script

Co-authored-by: Ubuntu <zain@zb-dev-vm.022x1jpnpm4u1iy2d325acts3c.yx.internal.cloudapp.net>

* Modify azp (sonic-net#9)

* Add libzmq5 as dependency for ubuntu-20.04 build

* Add libzmq3-dev dependency to build script

* Add uuid-dev and libboost-serialization-dev dependencies

Co-authored-by: Ubuntu <zain@zb-dev-vm.022x1jpnpm4u1iy2d325acts3c.yx.internal.cloudapp.net>

* Modify azp (sonic-net#10)

* Add libzmq5 as dependency for ubuntu-20.04 build

* Add libzmq3-dev dependency to build script

* Add uuid-dev and libboost-serialization-dev dependencies

* Add dependencies for bazel build

Co-authored-by: Ubuntu <zain@zb-dev-vm.022x1jpnpm4u1iy2d325acts3c.yx.internal.cloudapp.net>

* compile errors

* compile errors

* Compile error from Ubuntu

* drop unused macro

* compile fix

* compile fix

* remove pedantic

* restored ABORT_IF_NOT

* revert flag add

* comments correction

* Log every published event

* Install dependencies for vstest (sonic-net#15)

* Install deps vstest (sonic-net#16)

* Install dependencies for vstest

* Add comments

* Install deps vstest (sonic-net#17)

* Install dependencies for vstest

* Add comments

* Correct lib spelling

* Added LINGER timeout to service sockets

* Added heartbeat

* compile fix

* compile fix

* compile fix

* set LINGER_TIMEOUT before connect/bind

* Add global options

* Add global options

* Add global options

* Add global options

* Add global options

* Add global options

* Add global options

* Add global options

* Add global options

* added define

* Moved C API to C header file

* minor upates

* minor upates

* Updated comments

* comments update

* comments update

* Fix build issue

* Modify comment

* Modify comment

* Modify comment

* Modify comment

* Modify comment

* Modify comment

* Modify comment

Co-authored-by: Renuka Manavalan <[email protected]>
Co-authored-by: Renuka Manavalan <[email protected]>
Co-authored-by: Ubuntu <zain@zb-dev-vm.022x1jpnpm4u1iy2d325acts3c.yx.internal.cloudapp.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant