Skip to content

Modified kubectl-applogs plusing file and crlogs.py file#1374

Merged
devdattakulkarni merged 3 commits intocloud-ark:masterfrom
rahulkumar-choudhary:master
Mar 9, 2025
Merged

Modified kubectl-applogs plusing file and crlogs.py file#1374
devdattakulkarni merged 3 commits intocloud-ark:masterfrom
rahulkumar-choudhary:master

Conversation

@rahulkumar-choudhary
Copy link
Copy Markdown
Contributor

Fix #1372

Modified changes:

  • Removed connections parameter from the command python /$KUBEPLUS_HOME/plugins/crlogs.py $canonicalKind $instance $namespace $kubeconfig in kubectl-applogs plugin file.

  • Removed unnecessary code from crlogs.py file including the commented code and relation block for connections and composition.

To be modified:

Further to look into the clogs.py file. clogs.py file contains two unused functions:-

  1. get_resources_composition
  2. get_pods1

As per my understanding, we are not using relation block for connections and composition so the function get_resources_composition is unnecessary. Also, the function get_pods1 is not being used from the beginning. I have crosschecked - these two functions are not being used in any other files.

Please let me know on what should be done the these two functions. My suggestion would be - as we are not using these functions we shall remove it from the code base.

removed "connections" parameter from the command "python /$KUBEPLUS_HOME/plugins/crlogs.py $canonicalKind $instance $namespace $kubeconfig" in kubectl-applogs plugin file.

removed unnecessary code from crlogs.py file including the commented code and relation block for connections and composition.
@devdattakulkarni
Copy link
Copy Markdown
Contributor

@rahulkumar-choudhary Yes, let's remove the un-used functions.
Also, can you add a test to test the functioning of the app logs plugin.
See below for a comment to this effect.
https://github.com/cloud-ark/kubeplus/blob/master/tests/tests.py#L587

You can refer to the test_license_plugin (https://github.com/cloud-ark/kubeplus/blob/master/tests/tests.py#L89)
for reference.

The outline for the test will look as follow:

  • register a CRD
  • create an instance
  • run kubectl applogs
  • verify that there is no error
  • delete app instance
  • de-register the CRD

When creating an app instance, use a random name. We have been wanting to change our tests to use random names (check #1359). At least we should start doing this for new tests.

@devdattakulkarni devdattakulkarni merged commit 99ef27c into cloud-ark:master Mar 9, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kubectl applogs cleanup

2 participants