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

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rahulkumar-choudhary
Copy link

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
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.

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